From 1bd82ce508977f45470c74fdca53a28695cd5b8f Mon Sep 17 00:00:00 2001 From: l00520400 Date: Wed, 16 Feb 2022 14:39:57 +0800 Subject: [PATCH 1/2] review Signed-off-by: l00520400 Change-Id: I5413215700fc16696fd19964578881e7517871be Signed-off-by: l00520400 --- .../src/native_token_info_parcel.cpp | 5 +++- .../src/accesstoken_manager_client.cpp | 7 +++-- .../unittest/src/accesstoken_kit_test.cpp | 20 +++++++------- .../innerkits/nativetoken/src/nativetoken.c | 27 +++++++++---------- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/frameworks/accesstoken/src/native_token_info_parcel.cpp b/frameworks/accesstoken/src/native_token_info_parcel.cpp index 50795dc36..5b10bb77d 100644 --- a/frameworks/accesstoken/src/native_token_info_parcel.cpp +++ b/frameworks/accesstoken/src/native_token_info_parcel.cpp @@ -38,7 +38,10 @@ bool NativeTokenInfoParcel::Marshalling(Parcel& out) const RETURN_IF_FALSE(out.WriteUint32(this->nativeTokenInfoParams.tokenID)); RETURN_IF_FALSE(out.WriteUint32(this->nativeTokenInfoParams.tokenAttr)); - int dcapSize = (int)(this->nativeTokenInfoParams.dcap).size(); + if ((this->nativeTokenInfoParams.dcap).size() > INT32_MAX) { + return false; + } + int dcapSize = static_cast((this->nativeTokenInfoParams.dcap).size()); RETURN_IF_FALSE(out.WriteInt32(dcapSize)); for (auto dcapItem : this->nativeTokenInfoParams.dcap) { diff --git a/interfaces/innerkits/accesstoken/src/accesstoken_manager_client.cpp b/interfaces/innerkits/accesstoken/src/accesstoken_manager_client.cpp index f7e5d4f0d..fc9225c37 100644 --- a/interfaces/innerkits/accesstoken/src/accesstoken_manager_client.cpp +++ b/interfaces/innerkits/accesstoken/src/accesstoken_manager_client.cpp @@ -184,7 +184,7 @@ ATokenTypeEnum AccessTokenManagerClient::GetTokenType(AccessTokenID tokenID) ACCESSTOKEN_LOG_ERROR(LABEL, "proxy is null"); return TOKEN_INVALID; } - return (ATokenTypeEnum)(proxy->GetTokenType(tokenID)); + return static_cast(proxy->GetTokenType(tokenID)); } int AccessTokenManagerClient::CheckNativeDCap(AccessTokenID tokenID, const std::string& dcap) @@ -389,12 +389,11 @@ sptr AccessTokenManagerClient::GetProxy() return nullptr; } - auto proxy = iface_cast(accesstokenSa); - if (proxy == nullptr) { + proxy_ = iface_cast(accesstokenSa); + if (proxy_ == nullptr) { ACCESSTOKEN_LOG_DEBUG(LABEL, "iface_cast get null"); return nullptr; } - proxy_ = proxy; } } return proxy_; diff --git a/interfaces/innerkits/accesstoken/test/unittest/src/accesstoken_kit_test.cpp b/interfaces/innerkits/accesstoken/test/unittest/src/accesstoken_kit_test.cpp index c80c6301d..e3fca2f8a 100644 --- a/interfaces/innerkits/accesstoken/test/unittest/src/accesstoken_kit_test.cpp +++ b/interfaces/innerkits/accesstoken/test/unittest/src/accesstoken_kit_test.cpp @@ -26,14 +26,14 @@ using namespace OHOS::Security::AccessToken; namespace { static constexpr OHOS::HiviewDFX::HiLogLabel LABEL = {LOG_CORE, SECURITY_DOMAIN_ACCESSTOKEN, "AccessTokenKitTest"}; -static PermissionStateFull g_grantPermissionReq = { +PermissionStateFull g_grantPermissionReq = { .permissionName = "ohos.permission.GRANT_SENSITIVE_PERMISSIONS", .isGeneral = true, .resDeviceID = {"device"}, .grantStatus = {PermissionState::PERMISSION_GRANTED}, .grantFlags = {PermissionFlag::PERMISSION_SYSTEM_FIXED} }; -static PermissionStateFull g_revokePermissionReq = { +PermissionStateFull g_revokePermissionReq = { .permissionName = "ohos.permission.REVOKE_SENSITIVE_PERMISSIONS", .isGeneral = true, .resDeviceID = {"device"}, @@ -41,7 +41,7 @@ static PermissionStateFull g_revokePermissionReq = { .grantFlags = {PermissionFlag::PERMISSION_SYSTEM_FIXED} }; -static PermissionDef g_infoManagerTestPermDef1 = { +PermissionDef g_infoManagerTestPermDef1 = { .permissionName = "ohos.permission.test1", .bundleName = "accesstoken_test", .grantMode = 1, @@ -52,7 +52,7 @@ static PermissionDef g_infoManagerTestPermDef1 = { .availableLevel = APL_NORMAL }; -static PermissionDef g_infoManagerTestPermDef2 = { +PermissionDef g_infoManagerTestPermDef2 = { .permissionName = "ohos.permission.test2", .bundleName = "accesstoken_test", .grantMode = 1, @@ -63,7 +63,7 @@ static PermissionDef g_infoManagerTestPermDef2 = { .availableLevel = APL_NORMAL }; -static PermissionStateFull g_infoManagerTestState1 = { +PermissionStateFull g_infoManagerTestState1 = { .grantFlags = {1}, .grantStatus = {PermissionState::PERMISSION_GRANTED}, .isGeneral = true, @@ -71,7 +71,7 @@ static PermissionStateFull g_infoManagerTestState1 = { .resDeviceID = {"local"} }; -static PermissionStateFull g_infoManagerTestState2 = { +PermissionStateFull g_infoManagerTestState2 = { .permissionName = "ohos.permission.test2", .isGeneral = false, .grantFlags = {1, 2}, @@ -79,28 +79,28 @@ static PermissionStateFull g_infoManagerTestState2 = { .resDeviceID = {"device 1", "device 2"} }; -static HapInfoParams g_infoManagerTestInfoParms = { +HapInfoParams g_infoManagerTestInfoParms = { .bundleName = "accesstoken_test", .userID = 1, .instIndex = 0, .appIDDesc = "testtesttesttest" }; -static HapPolicyParams g_infoManagerTestPolicyPrams = { +HapPolicyParams g_infoManagerTestPolicyPrams = { .apl = APL_NORMAL, .domain = "test.domain", .permList = {g_infoManagerTestPermDef1, g_infoManagerTestPermDef2}, .permStateList = {g_infoManagerTestState1, g_infoManagerTestState2} }; -static HapInfoParams g_infoManagerTestInfoParmsBak = { +HapInfoParams g_infoManagerTestInfoParmsBak = { .bundleName = "accesstoken_test", .userID = 1, .instIndex = 0, .appIDDesc = "testtesttesttest" }; -static HapPolicyParams g_infoManagerTestPolicyPramsBak = { +HapPolicyParams g_infoManagerTestPolicyPramsBak = { .apl = APL_NORMAL, .domain = "test.domain", .permList = {g_infoManagerTestPermDef1, g_infoManagerTestPermDef2}, diff --git a/interfaces/innerkits/nativetoken/src/nativetoken.c b/interfaces/innerkits/nativetoken/src/nativetoken.c index f39d36eef..39a9b4a3d 100644 --- a/interfaces/innerkits/nativetoken/src/nativetoken.c +++ b/interfaces/innerkits/nativetoken/src/nativetoken.c @@ -21,7 +21,6 @@ int32_t g_isNativeTokenInited = 0; int32_t GetFileBuff(const char *cfg, char **retBuff) { struct stat fileStat; - int32_t ret; char filePath[PATH_MAX_LEN + 1] = {0}; if (realpath(cfg, filePath) == NULL) { @@ -39,12 +38,13 @@ int32_t GetFileBuff(const char *cfg, char **retBuff) return ATRET_FAILED; } - int32_t fileSize = (int32_t)fileStat.st_size; - if ((fileSize < 0) || (fileSize > MAX_JSON_FILE_LEN)) { + if ((fileStat.st_size < 0) || (fileStat.st_size > MAX_JSON_FILE_LEN)) { ACCESSTOKEN_LOG_ERROR("[ATLIB-%s]:stat file size is invalid.", __func__); return ATRET_FAILED; } + int32_t fileSize = (int32_t)fileStat.st_size; + FILE *cfgFd = fopen(filePath, "r"); if (cfgFd == NULL) { ACCESSTOKEN_LOG_ERROR("[ATLIB-%s]:fopen file failed.", __func__); @@ -58,19 +58,17 @@ int32_t GetFileBuff(const char *cfg, char **retBuff) return ATRET_FAILED; } - if (fread(buff, (size_t)fileSize, 1, cfgFd) != 1) { + if (fread(buff, fileSize, 1, cfgFd) != 1) { ACCESSTOKEN_LOG_ERROR("[ATLIB-%s]:fread failed.", __func__); free(buff); buff = NULL; - ret = ATRET_FAILED; - } else { - buff[fileSize] = '\0'; - *retBuff = buff; - ret = ATRET_SUCCESS; + fclose(cfgFd); + return ATRET_FAILED; } - + buff[fileSize] = '\0'; + *retBuff = buff; fclose(cfgFd); - return ret; + return ATRET_SUCCESS; } void FreeDcaps(char *dcaps[MAX_DCAPS_NUM], int32_t num) @@ -142,7 +140,7 @@ int32_t GetDcapsInfoFromJson(cJSON *cjsonItem, NativeTokenList *tokenNode) ACCESSTOKEN_LOG_ERROR("[ATLIB-%s]:dcapItem is invalid.", __func__); return ATRET_FAILED; } - tokenNode->dcaps[i] = (char *)malloc(sizeof(char) * length); + tokenNode->dcaps[i] = (char *)malloc(sizeof(char) * (length + 1)); if (tokenNode->dcaps[i] == NULL) { FreeDcaps(tokenNode->dcaps, i - 1); ACCESSTOKEN_LOG_ERROR("[ATLIB-%s]:malloc invalid.", __func__); @@ -165,6 +163,7 @@ int32_t GetTokenList(const cJSON *object) NativeTokenList *tmp = NULL; if (object == NULL) { + ACCESSTOKEN_LOG_ERROR("[ATLIB-%s]:object is null.", __func__); return ATRET_FAILED; } arraySize = cJSON_GetArraySize(object); @@ -195,7 +194,7 @@ int32_t GetTokenList(const cJSON *object) return ATRET_SUCCESS; } -int32_t ParseTokenInfoFromCfg(const char *filename) +int32_t ParseTokenInfo(const char *filename) { char *fileBuff = NULL; cJSON *record = NULL; @@ -230,7 +229,7 @@ int32_t AtlibInit(void) } g_tokenListHead->next = NULL; - int32_t ret = ParseTokenInfoFromCfg(TOKEN_ID_CFG_FILE_PATH); + int32_t ret = ParseTokenInfo(TOKEN_ID_CFG_FILE_PATH); if (ret != ATRET_SUCCESS) { free(g_tokenListHead); g_tokenListHead = NULL; -- Gitee From fcaec77542ae00729532544586a2e044cb681aa1 Mon Sep 17 00:00:00 2001 From: l00520400 Date: Wed, 16 Feb 2022 18:53:22 +0800 Subject: [PATCH 2/2] warning Signed-off-by: l00520400 Change-Id: Ie5092208774bce9178b639989ea62683649918b6 Signed-off-by: l00520400 --- frameworks/common/include/accesstoken_log.h | 1 - frameworks/common/include/data_validator.h | 4 ++-- frameworks/common/include/random.h | 2 +- .../main/cpp/include/token/native_token_info_inner.h | 2 +- .../main/cpp/src/token/accesstoken_id_manager.cpp | 3 --- 5 files changed, 4 insertions(+), 8 deletions(-) diff --git a/frameworks/common/include/accesstoken_log.h b/frameworks/common/include/accesstoken_log.h index ac4aaa107..9a859397d 100644 --- a/frameworks/common/include/accesstoken_log.h +++ b/frameworks/common/include/accesstoken_log.h @@ -51,7 +51,6 @@ static constexpr unsigned int SECURITY_DOMAIN_ACCESSTOKEN = 0xD002F01; #else -#include #include /* define LOG_TAG as "security_*" at your submodule, * means your submodule name such as "security_dac" */ diff --git a/frameworks/common/include/data_validator.h b/frameworks/common/include/data_validator.h index 8821598cf..5e69a4b84 100644 --- a/frameworks/common/include/data_validator.h +++ b/frameworks/common/include/data_validator.h @@ -12,12 +12,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#ifndef DATA_VALIDATOR_H +#define DATA_VALIDATOR_H #include #include "access_token.h" -#ifndef DATA_VALIDATOR_H -#define DATA_VALIDATOR_H namespace OHOS { namespace Security { namespace AccessToken { diff --git a/frameworks/common/include/random.h b/frameworks/common/include/random.h index 936276421..278cc24aa 100644 --- a/frameworks/common/include/random.h +++ b/frameworks/common/include/random.h @@ -22,7 +22,7 @@ extern "C" { #endif #endif -unsigned int GetRandomUint32(); +unsigned int GetRandomUint32(void); #ifdef __cplusplus #if __cplusplus diff --git a/services/accesstokenmanager/main/cpp/include/token/native_token_info_inner.h b/services/accesstokenmanager/main/cpp/include/token/native_token_info_inner.h index 527d176d1..9fc9a5b3b 100644 --- a/services/accesstokenmanager/main/cpp/include/token/native_token_info_inner.h +++ b/services/accesstokenmanager/main/cpp/include/token/native_token_info_inner.h @@ -16,10 +16,10 @@ #ifndef ACCESSTOKEN_NATIVE_TOKEN_INFO_INNER_H #define ACCESSTOKEN_NATIVE_TOKEN_INFO_INNER_H -#include "access_token.h" #include "native_token_info.h" #include #include +#include "access_token.h" #include "generic_values.h" namespace OHOS { diff --git a/services/accesstokenmanager/main/cpp/src/token/accesstoken_id_manager.cpp b/services/accesstokenmanager/main/cpp/src/token/accesstoken_id_manager.cpp index 7bedb7555..3d1421b82 100644 --- a/services/accesstokenmanager/main/cpp/src/token/accesstoken_id_manager.cpp +++ b/services/accesstokenmanager/main/cpp/src/token/accesstoken_id_manager.cpp @@ -14,9 +14,6 @@ */ #include "accesstoken_id_manager.h" - -#include - #include "accesstoken_log.h" #include "data_validator.h" #include "random.h" -- Gitee