From 0791b8eef08db8c5eaafbfb3f34fffa478489e0e Mon Sep 17 00:00:00 2001 From: RuiChen_01 Date: Thu, 18 Jun 2026 16:59:06 +0800 Subject: [PATCH] refactor(skill): carry callerTokenId via AbilityRequest instead of Want MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Permission checks (CheckStaticCfgPermission, CheckCallServiceExtensionPermission, CheckStartByCallPermission) used to read SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID straight out of the Want that flows through GenerateAbilityRequest. Because Want is attacker controlled on the public StartAbility paths, a forged callerTokenId could route the skill-specific permission branches in PermissionVerification (CheckSkillStartByCallPermission, JudgeInvisibleAndBackground) and borrow another app's identity. Add a dedicated AbilityRequest::skillCallerTokenId field populated solely by the trusted skill entrypoints: - ExecuteInAppSkill / ExecuteInAppSkillWithTokenId drop the want.SetParam calls and pass callerTokenId through to StartAbilityByCallWithSkill / StartExtensionAbilityWithSkill. - Those two helpers (and StartExtensionAbilityInner via a new optional parameter) set the AbilityRequest field after the request is generated. - The three permission checks now read abilityRequest.skillCallerTokenId instead of the Want. Forged Want params no longer influence permission decisions. Existing callers of StartExtensionAbilityInner are unaffected via the default parameter value. Co-Authored-By: Agent Signed-off-by: RuiChen_01 🤖‍ AI[100%] 👌 AI Adopted[100%] 🧑 Human[0%] Co-authored-by: claude (glm-5.2) --- .../include/ability_manager_service.h | 9 +++-- .../include/ability_record/ability_request.h | 1 + .../src/ability_manager_service.cpp | 33 +++++++++---------- .../src/skill/skill_execute_manager.cpp | 4 +-- .../ability_manager_service_first_test.cpp | 6 ++-- 5 files changed, 26 insertions(+), 27 deletions(-) diff --git a/services/abilitymgr/include/ability_manager_service.h b/services/abilitymgr/include/ability_manager_service.h index 69c9527ca8..95f2d01b0d 100644 --- a/services/abilitymgr/include/ability_manager_service.h +++ b/services/abilitymgr/include/ability_manager_service.h @@ -1442,7 +1442,8 @@ public: bool checkSystemCaller = true, bool isImplicit = false, bool isDlp = false, - bool isStartAsCaller = false); + bool isStartAsCaller = false, + uint32_t skillCallerTokenId = 0); int RequestModalUIExtensionInner(Want want); @@ -2230,9 +2231,11 @@ public: const sptr &callback) override; int32_t StartAbilityByCallWithSkill(const Want &want, - const sptr &callerToken, int32_t userId = DEFAULT_INVAL_VALUE); + const sptr &callerToken, int32_t userId = DEFAULT_INVAL_VALUE, + uint32_t skillCallerTokenId = 0); - int32_t StartExtensionAbilityWithSkill(const Want &want, int32_t userId); + int32_t StartExtensionAbilityWithSkill(const Want &want, int32_t userId, + uint32_t skillCallerTokenId = 0); int32_t ExecuteSkillDone(const sptr &token, const std::string &requestCode, int32_t resultCode, const AppExecFwk::SkillExecuteResult &result) override; diff --git a/services/abilitymgr/include/ability_record/ability_request.h b/services/abilitymgr/include/ability_record/ability_request.h index eab949eb8b..ebe3fe4620 100644 --- a/services/abilitymgr/include/ability_record/ability_request.h +++ b/services/abilitymgr/include/ability_record/ability_request.h @@ -90,6 +90,7 @@ struct AbilityRequest { int32_t loadExtensionTimeout = 0; // only for connectAbility uint32_t callerAccessTokenId = 0; uint32_t specifyTokenId = 0; + uint32_t skillCallerTokenId = 0; int callerUid = -1; // call ability int requestCode = -1; int32_t atomicServiceShortLink = 0; diff --git a/services/abilitymgr/src/ability_manager_service.cpp b/services/abilitymgr/src/ability_manager_service.cpp index a04e84aea5..ef13975e53 100644 --- a/services/abilitymgr/src/ability_manager_service.cpp +++ b/services/abilitymgr/src/ability_manager_service.cpp @@ -4340,7 +4340,7 @@ bool AbilityManagerService::CheckWorkSchedulerPermission(const sptr &callerToken, int32_t userId, AppExecFwk::ExtensionAbilityType extensionType, bool checkSystemCaller, bool isImplicit, - bool isDlp, bool isStartAsCaller) + bool isDlp, bool isStartAsCaller, uint32_t skillCallerTokenId) { HITRACE_METER_NAME(HITRACE_TAG_ABILITY_MANAGER, __PRETTY_FUNCTION__); TAG_LOGI(AAFwkTag::SERVICE_EXT, @@ -4450,6 +4450,7 @@ int32_t AbilityManagerService::StartExtensionAbilityInner(const Want &want, cons return result; } + abilityRequest.skillCallerTokenId = skillCallerTokenId; abilityRequest.userId = validUserId; if (!HandleExecuteSAInterceptor(want, callerToken, abilityRequest, result)) { return result; @@ -11650,8 +11651,7 @@ int AbilityManagerService::CheckStaticCfgPermission(const AppExecFwk::AbilityReq if (specifyTokenId != 0) { isSaCall = AAFwk::PermissionVerification::GetInstance()->IsSACallByTokenId(specifyTokenId); } - uint32_t skillCallerTokenId = static_cast( - abilityRequest.want.GetIntParam(AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, 0)); + uint32_t skillCallerTokenId = abilityRequest.skillCallerTokenId; if (isSaCall) { // do not need check static config permission when start ability by SA return AppExecFwk::Constants::PERMISSION_GRANTED; @@ -12525,8 +12525,7 @@ int AbilityManagerService::CheckCallServiceExtensionPermission(const AbilityRequ verificationInfo.specifyTokenId = (abilityRequest.specifiedFullTokenId != 0) ? static_cast(abilityRequest.specifiedFullTokenId) : static_cast(abilityRequest.specifyTokenId); - verificationInfo.skillCallerTokenId = static_cast( - abilityRequest.want.GetIntParam(AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, 0)); + verificationInfo.skillCallerTokenId = abilityRequest.skillCallerTokenId; if (isParamStartAbilityEnable_) { bool stopContinuousTaskFlag = ShouldPreventStartAbility(abilityRequest); if (stopContinuousTaskFlag) { @@ -12993,8 +12992,7 @@ int AbilityManagerService::CheckStartByCallPermission(const AbilityRequest &abil verificationInfo.visible = abilityRequest.abilityInfo.visible; verificationInfo.withContinuousTask = IsBackgroundTaskUid(IPCSkeleton::GetCallingUid()); verificationInfo.specifiedFullTokenId = static_cast(abilityRequest.specifiedFullTokenId); - verificationInfo.skillCallerTokenId = static_cast( - abilityRequest.want.GetIntParam(AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, 0)); + verificationInfo.skillCallerTokenId = abilityRequest.skillCallerTokenId; if (IsCallFromBackground(abilityRequest, verificationInfo.isBackgroundCall, false) != ERR_OK) { return ERR_INVALID_VALUE; @@ -14693,12 +14691,11 @@ int32_t AbilityManagerService::ExecuteInAppSkill(const std::string &bundleName, } // 5. Launch target based on type - want.SetParam(AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, - static_cast(IPCSkeleton::GetCallingTokenID())); + uint32_t skillCallerTokenId = IPCSkeleton::GetCallingTokenID(); if (targetType == AppExecFwk::ExtensionAbilityType::SERVICE) { - return StartExtensionAbilityWithSkill(want, userId); + return StartExtensionAbilityWithSkill(want, userId, skillCallerTokenId); } - return StartAbilityByCallWithSkill(want, nullptr, userId); + return StartAbilityByCallWithSkill(want, nullptr, userId, skillCallerTokenId); } int32_t AbilityManagerService::ExecuteInAppSkillWithTokenId(const AppExecFwk::SkillExecuteRequest &request, @@ -14749,16 +14746,14 @@ int32_t AbilityManagerService::ExecuteInAppSkillWithTokenId(const AppExecFwk::Sk } // 5. Launch target based on type - want.SetParam(AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, - static_cast(request.callerTokenId)); if (targetType == AppExecFwk::ExtensionAbilityType::SERVICE) { - return StartExtensionAbilityWithSkill(want, userId); + return StartExtensionAbilityWithSkill(want, userId, request.callerTokenId); } - return StartAbilityByCallWithSkill(want, nullptr, userId); + return StartAbilityByCallWithSkill(want, nullptr, userId, request.callerTokenId); } int32_t AbilityManagerService::StartAbilityByCallWithSkill(const Want &want, - const sptr &callerToken, int32_t userId) + const sptr &callerToken, int32_t userId, uint32_t skillCallerTokenId) { TAG_LOGI(AAFwkTag::ABILITYMGR, "start ability by call with skill intent"); sptr connect = sptr::MakeSptr(); @@ -14780,6 +14775,7 @@ int32_t AbilityManagerService::StartAbilityByCallWithSkill(const Want &want, TAG_LOGE(AAFwkTag::ABILITYMGR, "generate ability request error"); return result; } + abilityRequest.skillCallerTokenId = skillCallerTokenId; std::shared_ptr targetRecord; if (IsAbilityStarted(abilityRequest, targetRecord, oriValidUserId)) { @@ -14798,11 +14794,12 @@ int32_t AbilityManagerService::StartAbilityByCallWithSkill(const Want &want, return result; } -int32_t AbilityManagerService::StartExtensionAbilityWithSkill(const Want &want, int32_t userId) +int32_t AbilityManagerService::StartExtensionAbilityWithSkill(const Want &want, int32_t userId, + uint32_t skillCallerTokenId) { TAG_LOGI(AAFwkTag::ABILITYMGR, "start extension ability with skill intent"); return StartExtensionAbilityInner(want, nullptr, userId, - AppExecFwk::ExtensionAbilityType::SERVICE, true); + AppExecFwk::ExtensionAbilityType::SERVICE, true, false, false, false, skillCallerTokenId); } int32_t AbilityManagerService::ExecuteSkillDone(const sptr &token, diff --git a/services/abilitymgr/src/skill/skill_execute_manager.cpp b/services/abilitymgr/src/skill/skill_execute_manager.cpp index da9231f6c8..d6c29fbdf1 100644 --- a/services/abilitymgr/src/skill/skill_execute_manager.cpp +++ b/services/abilitymgr/src/skill/skill_execute_manager.cpp @@ -88,8 +88,8 @@ int32_t SkillExecuteManager::CheckSkillPermission(const AppExecFwk::SkillInfo &s uint32_t callerTokenId) { TAG_LOGI(AAFwkTag::ABILITYMGR, - "check skill permission, skill:%{public}s callerTokenId:%{public}u", - skillInfo.skillName.c_str(), callerTokenId); + "check skill permission, skill:%{public}s", + skillInfo.skillName.c_str()); auto permVerif = PermissionVerification::GetInstance(); if (!permVerif->IsSACall()) { diff --git a/test/unittest/ability_manager_service_first_test/ability_manager_service_first_test.cpp b/test/unittest/ability_manager_service_first_test/ability_manager_service_first_test.cpp index 5d25ef675f..611514cc7c 100644 --- a/test/unittest/ability_manager_service_first_test/ability_manager_service_first_test.cpp +++ b/test/unittest/ability_manager_service_first_test/ability_manager_service_first_test.cpp @@ -2243,8 +2243,7 @@ HWTEST_F(AbilityManagerServiceFirstTest, CheckStaticCfgPermission_SkillCaller_00 auto abilityMs = std::make_shared(); AppExecFwk::AbilityRequest abilityRequest; abilityRequest.abilityInfo.permissions.push_back("test.permission"); - abilityRequest.want.SetParam( - AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, static_cast(100)); + abilityRequest.skillCallerTokenId = 100; MyFlag::flag_ = MyFlag::IS_SA_CALL; int ret = abilityMs->CheckStaticCfgPermission( abilityRequest, false, 0, false, false, false); @@ -2263,8 +2262,7 @@ HWTEST_F(AbilityManagerServiceFirstTest, CheckStaticCfgPermission_SkillCaller_00 auto abilityMs = std::make_shared(); AppExecFwk::AbilityRequest abilityRequest; uint32_t callerTokenId = 100; - abilityRequest.want.SetParam( - AppExecFwk::SKILL_EXECUTE_PARAM_CALLER_TOKEN_ID, static_cast(callerTokenId)); + abilityRequest.skillCallerTokenId = callerTokenId; abilityRequest.abilityInfo.applicationInfo.accessTokenId = callerTokenId; MyFlag::flag_ = 0; int ret = abilityMs->CheckStaticCfgPermission(