!2391 Fix security issues with appID

Merge pull request !2391 from 王斌/master

services/distributeddataservice/app/src/checker/bundle_checker.cpp

@@ -17,9 +17,14 @@
 #include "bundle_checker.h"
 #include <memory>
 #include "accesstoken_kit.h"
+#include "bundlemgr/bundle_mgr_proxy.h"
 #include "hap_token_info.h"
+#include "ipc_skeleton.h"
+#include "iservice_registry.h"
 #include "log_print.h"
+#include "system_ability_definition.h"
 #include "utils/crypto.h"
+
 namespace OHOS {
 namespace DistributedData {
 using namespace Security::AccessToken;
@@ -56,27 +61,47 @@ bool BundleChecker::SetSwitchesInfo(const CheckerManager::Switches &switches)
     return true;
 }
 
+std::string BundleChecker::GetBundleAppId(const CheckerManager::StoreInfo &info)
+{
+    auto samgrProxy = SystemAbilityManagerClient::GetInstance().GetSystemAbilityManager();
+    if (samgrProxy == nullptr) {
+        ZLOGE("Failed to get system ability mgr.");
+        return "";
+    }
+    auto bundleMgrProxy = samgrProxy->GetSystemAbility(BUNDLE_MGR_SERVICE_SYS_ABILITY_ID);
+    if (bundleMgrProxy == nullptr) {
+        ZLOGE("Failed to Get BMS SA.");
+        return "";
+    }
+    auto bundleManager = iface_cast<AppExecFwk::IBundleMgr>(bundleMgrProxy);
+    if (bundleManager == nullptr) {
+        ZLOGE("Failed to get bundle manager");
+        return "";
+    }
+    int32_t userId = info.uid / OHOS::AppExecFwk::Constants::BASE_USER_RANGE;
+    std::string appId = bundleManager->GetAppIdByBundleName(info.bundleName, userId);
+    if (appId.empty()) {
+        ZLOGE("GetAppIdByBundleName failed appId:%{public}s, bundleName:%{public}s, uid:%{public}d",
+            appId.c_str(), info.bundleName.c_str(), userId);
+    }
+    return appId;
+}
+
 std::string BundleChecker::GetAppId(const CheckerManager::StoreInfo &info)
 {
     if (AccessTokenKit::GetTokenTypeFlag(info.tokenId) != TOKEN_HAP) {
         return "";
     }
-    HapTokenInfo tokenInfo;
-    auto result = AccessTokenKit::GetHapTokenInfo(info.tokenId, tokenInfo);
-    if (result != RET_SUCCESS) {
-        ZLOGE("token:0x%{public}x, result:%{public}d", info.tokenId, result);
-        return "";
-    }
-    if (!info.bundleName.empty() && tokenInfo.bundleName != info.bundleName) {
-        ZLOGE("bundlename:%{public}s <-> %{public}s", info.bundleName.c_str(), tokenInfo.bundleName.c_str());
+    auto appId = GetBundleAppId(info);
+    if (appId.empty()) {
         return "";
     }
     auto it = trusts_.find(info.bundleName);
-    if (it != trusts_.end() && (it->second == tokenInfo.appID)) {
+    if (it != trusts_.end() && (it->second == appId)) {
         return info.bundleName;
     }
-    ZLOGD("bundleName:%{public}s, appId:%{public}s", info.bundleName.c_str(), tokenInfo.appID.c_str());
-    return Crypto::Sha256(tokenInfo.appID);
+    ZLOGD("bundleName:%{public}s, appId:%{public}s", info.bundleName.c_str(), appId.c_str());
+    return Crypto::Sha256(appId);
 }
 
 bool BundleChecker::IsValid(const CheckerManager::StoreInfo &info)
@@ -98,18 +123,12 @@ bool BundleChecker::IsDistrust(const CheckerManager::StoreInfo &info)
     if (AccessTokenKit::GetTokenTypeFlag(info.tokenId) != TOKEN_HAP) {
         return false;
     }
-    HapTokenInfo tokenInfo;
-    auto result = AccessTokenKit::GetHapTokenInfo(info.tokenId, tokenInfo);
-    if (result != RET_SUCCESS) {
-        ZLOGE("token:0x%{public}x, result:%{public}d", info.tokenId, result);
-        return false;
-    }
-    if (!info.bundleName.empty() && tokenInfo.bundleName != info.bundleName) {
-        ZLOGE("bundlename:%{public}s <-> %{public}s", info.bundleName.c_str(), tokenInfo.bundleName.c_str());
+    auto appId = GetBundleAppId(info);
+    if (appId.empty()) {
         return false;
     }
     auto it = distrusts_.find(info.bundleName);
-    if (it != distrusts_.end() && (it->second == tokenInfo.appID)) {
+    if (it != distrusts_.end() && (it->second == appId)) {
         return true;
     }
     return false;

services/distributeddataservice/app/src/checker/bundle_checker.h

@@ -44,6 +44,7 @@ private:
     std::map<std::string, std::string> switches_;
     std::vector<CheckerManager::StoreInfo> dynamicStores_;
     std::vector<CheckerManager::StoreInfo> staticStores_;
+    std::string GetBundleAppId(const CheckerManager::StoreInfo &info);
 };
 } // namespace DistributedData
 } // namespace OHOS

services/distributeddataservice/framework/test/checker_manager_test.cpp

@@ -153,25 +153,6 @@ HWTEST_F(CheckerManagerTest, SystemCheckerIVI, TestSize.Level0)
     ASSERT_TRUE(CheckerManager::GetInstance().IsValid(info));
 }
 
-/**
-* @tc.name: BundleChecker
-* @tc.desc: checker the bundle name of the bundle abilities.
-* @tc.type: FUNC
-* @tc.require:
-* @tc.author: Sven Wang
-*/
-HWTEST_F(CheckerManagerTest, BundleChecker, TestSize.Level0)
-{
-    CheckerManager::StoreInfo storeInfo;
-    storeInfo.uid = 2000000;
-    storeInfo.tokenId = AccessTokenKit::GetHapTokenID(100, "ohos.test.demo", 0);
-    storeInfo.bundleName = "ohos.test.demo";
-    HapTokenInfo tokenInfo;
-    AccessTokenKit::GetHapTokenInfo(storeInfo.tokenId, tokenInfo);
-    ASSERT_EQ(Crypto::Sha256(tokenInfo.appID), CheckerManager::GetInstance().GetAppId(storeInfo));
-    ASSERT_TRUE(CheckerManager::GetInstance().IsValid(storeInfo));
-}
-
 /**
 * @tc.name: IsDynamic
 * @tc.desc: checker data type.

services/distributeddataservice/service/test/fuzztest/cloudservicestub_fuzzer/BUILD.gn

@@ -128,6 +128,8 @@ ohos_fuzztest("CloudServiceStubFuzzTest") {
     "kv_store:distributeddata_inner",
     "kv_store:distributeddata_mgr",
     "relational_store:native_rdb",
+    "safwk:system_ability_fwk",
+    "samgr:samgr_proxy",
   ]
 }
 

services/distributeddataservice/service/test/fuzztest/kvdbservicestub_fuzzer/BUILD.gn

@@ -117,6 +117,8 @@ ohos_fuzztest("KvdbServiceStubFuzzTest") {
     "ipc:ipc_core",
     "kv_store:distributeddata_inner",
     "kv_store:distributeddata_mgr",
+    "safwk:system_ability_fwk",
+    "samgr:samgr_proxy",
   ]
 }