From 862f950c2548e138c2f2aab0678032ed8d21f4f5 Mon Sep 17 00:00:00 2001 From: Ivan Pazderskyy Date: Wed, 2 Mar 2016 17:19:49 +0200 Subject: [PATCH] Fix issues reported by SVACE tool in CKM module. Change-Id: I995b4b6f0f58f9db0be2eb9703a80052b69523b2 Signed-off-by: i.pazderskyy Reviewed-on: https://gerrit.iotivity.org/gerrit/5311 Tested-by: jenkins-iotivity Reviewed-by: Dmitriy Zhuravlev Reviewed-by: Randeep Singh --- .../provisioning/ck_manager/sample/Door_sample.cpp | 8 ++- .../ck_manager/sample/provisioningclient.c | 68 ++++++++++++++++++++-- .../provisioning/ck_manager/src/ck_manager.c | 4 +- .../provisioning/ck_manager/src/ckm_info.c | 10 ++-- .../provisioning/ck_manager/src/crl_generator.c | 2 +- .../provisioning/src/credentialgenerator.c | 8 +-- 6 files changed, 81 insertions(+), 19 deletions(-) diff --git a/resource/csdk/security/provisioning/ck_manager/sample/Door_sample.cpp b/resource/csdk/security/provisioning/ck_manager/sample/Door_sample.cpp index ca1df02..9425b5f 100755 --- a/resource/csdk/security/provisioning/ck_manager/sample/Door_sample.cpp +++ b/resource/csdk/security/provisioning/ck_manager/sample/Door_sample.cpp @@ -466,7 +466,7 @@ void *input_function(void * /*data*/) cbData.context = (void *)DEFAULT_CONTEXT_VALUE; cbData.cd = NULL; - strcpy(szQueryUri, MULTICAST_DISCOVERY_QUERY); + strncpy(szQueryUri, MULTICAST_DISCOVERY_QUERY, sizeof(szQueryUri)); while (1) { @@ -478,7 +478,11 @@ void *input_function(void * /*data*/) if (isUpdated == false) { OIC_LOG(INFO, TAG, "isUpdated is false..."); - OCDoResource(&handle, OC_REST_DISCOVER, szQueryUri, 0, 0, CT_DEFAULT, OC_LOW_QOS, &cbData, NULL, 0); + if (OCDoResource(&handle, OC_REST_DISCOVER, szQueryUri, 0, 0, CT_DEFAULT, + OC_LOW_QOS, &cbData, NULL, 0) != OC_STACK_OK) + { + OIC_LOG(ERROR, TAG, "OCDoResource error"); + } } break; diff --git a/resource/csdk/security/provisioning/ck_manager/sample/provisioningclient.c b/resource/csdk/security/provisioning/ck_manager/sample/provisioningclient.c index 6fec1da..8d9aed1 100644 --- a/resource/csdk/security/provisioning/ck_manager/sample/provisioningclient.c +++ b/resource/csdk/security/provisioning/ck_manager/sample/provisioningclient.c @@ -205,7 +205,7 @@ static int InputACL(OicSecAcl_t *acl) { if (DASH != temp_id[i]) { - if(j>UUID_LENGTH) + if(j >= UUID_LENGTH) { printf("Invalid input\n"); return -1; @@ -217,6 +217,11 @@ static int InputACL(OicSecAcl_t *acl) //Set Resource. printf("Num. of Resource : \n"); ret = scanf("%zu", &acl->resourcesLen); + if(-1 == ret) + { + printf("Error while input\n"); + return -1; + } printf("-URI of resource\n"); printf("ex) /a/light (Max_URI_Length: 64 Byte )\n"); acl->resources = (char **)OICCalloc(acl->resourcesLen, sizeof(char *)); @@ -272,6 +277,11 @@ static int InputACL(OicSecAcl_t *acl) // Set Rowner printf("Num. of Rowner : "); ret = scanf("%zu", &acl->ownersLen); + if(-1 == ret) + { + printf("Error while input\n"); + return -1; + } printf("-URN identifying the rowner\n"); printf("ex) lightDeviceUUID0 (16 Numbers except to '-')\n"); acl->owners = (OicUuid_t *)OICCalloc(acl->ownersLen, sizeof(OicUuid_t)); @@ -446,19 +456,43 @@ static int InputCRL(OicSecCrl_t *crlRes) // const uint8_t revocationDatesContent[MAX_Revoked_NUMBER][DATE_LENGTH]; uint32_t nuberOfRevoked = 0; printf("Enter number of Revoked certificates(1..%d)\n", MAX_Revoked_NUMBER); - scanf("%u", &nuberOfRevoked); + int ret = 0; + ret = scanf("%u", &nuberOfRevoked); + if(-1 == ret) + { + printf("Error while input\n"); + return PKI_UNKNOWN_ERROR; + } + + if((uint32_t)MAX_Revoked_NUMBER < nuberOfRevoked) + { + OIC_LOG(ERROR, TAG, "Wrong revoked certificate number"); + return PKI_UNKNOWN_ERROR; + } for (size_t i = 0; i < nuberOfRevoked; ++i) { printf("Revoked certificate %d:", i); printf("Serial number (E. g.: 100):"); - scanf("%u", &revokedNumbers[i]); + ret = scanf("%u", &revokedNumbers[i]); + if(-1 == ret) + { + printf("Error while input\n"); + return PKI_UNKNOWN_ERROR; + } + revocationDates[i] = (const uint8_t*)"130101000005Z"; } crl.len = CRL_MIN_SIZE + nuberOfRevoked * (sizeof(CertificateRevocationInfo_t) + 4)/* + 1000*/; crl.data = (uint8_t *)OICCalloc(1, crl.len); + if (NULL == crl.data) + { + OIC_LOG(ERROR, TAG, "Error while memory allocation"); + return PKI_MEMORY_ALLOC_FAILED; + } + CHECK_CALL(CKMIssueCRL, uint8ThisUpdateTime, nuberOfRevoked, revokedNumbers, revocationDates, &crl); PRINT_BYTE_ARRAY("CRL:\n",crl); @@ -586,13 +620,30 @@ int main() int Device1 = 0; int Device2 = 0; + int ret = 0; printf("Select 2 devices for Credential & ACL provisioning\n"); printf("Device 1: "); - scanf("%d", &Device1); + ret = scanf("%d", &Device1); + if(-1 == ret) + { + printf("Error while input\n"); + goto error; + } + printf("Device 2: "); - scanf("%d", &Device2); + ret = scanf("%d", &Device2); + if(-1 == ret) + { + printf("Error while input\n"); + goto error; + } + if( 0 > Device1 || 0 > Device2 || Device1 > nOwnedDevice || Device2 > nOwnedDevice) + { + OIC_LOG(ERROR, TAG, "Wrong devices number"); + goto error; + } gAcl = (OicSecAcl_t *)OICCalloc(1,sizeof(OicSecAcl_t)); if (NULL == gAcl) @@ -669,6 +720,13 @@ int main() sleep(1); } gCrl = (OicSecCrl_t *)OICMalloc(sizeof(OicSecCrl_t)); + + if (NULL == gCrl) + { + OIC_LOG(ERROR, TAG, "Error while memory allocation"); + goto error; + } + if (PKI_SUCCESS != InputCRL(gCrl)) { OIC_LOG(ERROR, TAG, "CA init error"); diff --git a/resource/csdk/security/provisioning/ck_manager/src/ck_manager.c b/resource/csdk/security/provisioning/ck_manager/src/ck_manager.c index 30f07a0..25c579b 100644 --- a/resource/csdk/security/provisioning/ck_manager/src/ck_manager.c +++ b/resource/csdk/security/provisioning/ck_manager/src/ck_manager.c @@ -197,7 +197,6 @@ PKIError CKMIssueDeviceCertificate (const uint8_t *uint8SubjectName, uint8_t caPrivateKey[PRIVATE_KEY_SIZE]; uint8_t uint8caName[ISSUER_MAX_NAME_SIZE]; - CHECK_NULL(uint8SubjectPublicKey, ISSUER_NULL_PASSED); CHECK_NULL(issuedCertificate, ISSUER_NULL_PASSED); CHECK_NULL(issuedCertificate->data, ISSUER_NULL_PASSED); CHECK_LESS_EQUAL(ISSUER_MAX_CERT_SIZE, issuedCertificate->len, ISSUER_WRONG_BYTE_ARRAY_LEN); @@ -425,6 +424,7 @@ PKIError GenerateCSR (const uint8_t *uint8SubjectName, FUNCTION_CLEAR( OICFree(subjectName); + OICFree(subjectPublicKey->buf); OICFree(subjectPublicKey); OICFree(subjectPrivateKey->buf); OICFree(subjectPrivateKey); @@ -582,7 +582,7 @@ PKIError CKMRevocateCertificate (const uint8_t *uint8ThisUpdateTime, const long CHECK_CALL(InitCKMInfo); CHECK_CALL(GetNumberOfRevoked, &numberOfRevoked); - crlMaxSize = (CRL_MIN_SIZE + + crlMaxSize = (uint32_t)(CRL_MIN_SIZE + (numberOfRevoked + 1) * (sizeof(CertificateRevocationInfo_t) + 4)); CHECK_NULL(encodedCRL, ISSUER_NULL_PASSED); diff --git a/resource/csdk/security/provisioning/ck_manager/src/ckm_info.c b/resource/csdk/security/provisioning/ck_manager/src/ckm_info.c index 3820aeb..bf528b6 100644 --- a/resource/csdk/security/provisioning/ck_manager/src/ckm_info.c +++ b/resource/csdk/security/provisioning/ck_manager/src/ckm_info.c @@ -237,7 +237,7 @@ PKIError SetCAName (const ByteArray *CAName) CHECK_NULL_BYTE_ARRAY_PTR(CAName, ISSUER_CA_STORAGE_NULL_PASSED); CHECK_LESS_EQUAL(CAName->len, ISSUER_MAX_NAME_SIZE, ISSUER_CA_STORAGE_WRONG_CA_NAME_LEN); memcpy(g_ckmInfo.CAName, CAName->data, CAName->len); - g_ckmInfo.CANameSize = CAName->len; + g_ckmInfo.CANameSize = (uint32_t)CAName->len; FUNCTION_CLEAR(); } @@ -297,7 +297,7 @@ PKIError InitCRT(void) for (int i = 0; i < g_ckmInfo.CAChainLength; i++) { - objectsRead = fread(prefix, sizeof(uint8_t), CERT_LEN_PREFIX, filePointer); + objectsRead = (uint32_t)fread(prefix, sizeof(uint8_t), CERT_LEN_PREFIX, filePointer); CHECK_EQUAL(objectsRead, CERT_LEN_PREFIX, ISSUER_CA_STORAGE_CRT_READ_ERROR); g_ckmInfo.CACertificateChain[i].len = ParseCertPrefix(prefix); @@ -305,7 +305,7 @@ PKIError InitCRT(void) (uint8_t *)OICMalloc(g_ckmInfo.CACertificateChain[i].len); CHECK_NULL(g_ckmInfo.CACertificateChain[i].data, ISSUER_CA_STORAGE_MEMORY_ALLOC_FAILED); - objectsRead = fread(g_ckmInfo.CACertificateChain[i].data, sizeof(uint8_t), + objectsRead = (uint32_t)fread(g_ckmInfo.CACertificateChain[i].data, sizeof(uint8_t), g_ckmInfo.CACertificateChain[i].len, filePointer); CHECK_EQUAL(objectsRead, g_ckmInfo.CACertificateChain[i].len, ISSUER_CA_STORAGE_CRT_READ_ERROR); @@ -333,9 +333,9 @@ PKIError SaveCRT(void) for (int i = 0; i < g_ckmInfo.CAChainLength; i++) { WriteCertPrefix(prefix, g_ckmInfo.CACertificateChain[i].len); - objectsWrote = fwrite(prefix, sizeof(uint8_t), CERT_LEN_PREFIX, filePointer); + objectsWrote = (uint32_t)fwrite(prefix, sizeof(uint8_t), CERT_LEN_PREFIX, filePointer); CHECK_EQUAL(objectsWrote, CERT_LEN_PREFIX, ISSUER_CA_STORAGE_CRT_WRITE_ERROR); - objectsWrote = fwrite(g_ckmInfo.CACertificateChain[i].data, sizeof(uint8_t), + objectsWrote = (uint32_t)fwrite(g_ckmInfo.CACertificateChain[i].data, sizeof(uint8_t), g_ckmInfo.CACertificateChain[i].len, filePointer); CHECK_EQUAL(objectsWrote, g_ckmInfo.CACertificateChain[i].len, ISSUER_CA_STORAGE_CRT_WRITE_ERROR); diff --git a/resource/csdk/security/provisioning/ck_manager/src/crl_generator.c b/resource/csdk/security/provisioning/ck_manager/src/crl_generator.c index 710f8db..e6b0666 100644 --- a/resource/csdk/security/provisioning/ck_manager/src/crl_generator.c +++ b/resource/csdk/security/provisioning/ck_manager/src/crl_generator.c @@ -42,7 +42,7 @@ PKIError GenerateCRL (const UTF8String_t *issuerName, RelativeDistinguishedName_t *issuerRDN = NULL; CertificateRevocationInfo_t *cri = NULL; - uint32_t crlMaxSize = (CRL_MIN_SIZE + + uint32_t crlMaxSize = (uint32_t)(CRL_MIN_SIZE + numberOfRevoked * (sizeof(CertificateRevocationInfo_t) + 4)); uint32_t i; diff --git a/resource/csdk/security/provisioning/src/credentialgenerator.c b/resource/csdk/security/provisioning/src/credentialgenerator.c index f592c93..747ae37 100644 --- a/resource/csdk/security/provisioning/src/credentialgenerator.c +++ b/resource/csdk/security/provisioning/src/credentialgenerator.c @@ -169,7 +169,7 @@ static char *CreateCertificatePublicJWK(const char *const *certificateChain, size_t offset = strlen(firstPart); for (size_t i = 0; i < chainLength; ++i) { - offset += sprintf(certPubJWK + offset, "\"%s\",", certificateChain[i]); + offset += snprintf(certPubJWK + offset, certPubJWKLen, "\"%s\",", certificateChain[i]); } sprintf(certPubJWK + offset - 1, secondPart); } @@ -195,12 +195,12 @@ static char *CreateCertificatePrivateJWK(const char *privateKey) } const char firstPart[] = "{\"kty\":\"EC\",\"crv\":\"P-256\",\"d\":\""; const char secondPart[] = "\"}"; - char *certPrivJWK = (char *)OICMalloc(strlen(firstPart) + strlen(secondPart) + strlen( - privateKey) + 1); + size_t len = strlen(firstPart) + strlen(secondPart) + strlen(privateKey) + 1; + char *certPrivJWK = (char *)OICMalloc(len); if (NULL != certPrivJWK) { - sprintf(certPrivJWK, "%s%s%s", firstPart, privateKey, secondPart); + snprintf(certPrivJWK, len, "%s%s%s", firstPart, privateKey, secondPart); } else { -- 2.7.4