Fix code review issues

The main issues are concentrated on: the strictness of CMS attributes, the PBKDF2 iteration count for `enc` decryption file headers, the length estimation of stream-based Base64 encoding and decoding, the length accumulation of `BSL_ParamMaker`, the input length accumulation of PAKE KDF, the length accumulation of SPAKE2+ transcript, the UIO handle leakage in configuration files, and the issuance policy risk of `x509 -copy_extensions copyall`.
Multiple alerts such as PAKE register stack overflow and double-free, ElGamal output length, one-time Base64 encoding, etc., are no longer valid in the current source code. It is suspected that these are false positives caused by old code results or repeated/truncated automated reports.

Cherry-picked from: https://gitcode.com/openHiTLS/openhitls/merge_requests/1394

Cherry-picked from: https://gitcode.com/openHiTLS/openhitls/merge_requests/1407

Signed-off-by: Dongjianwei001 <dongjianwei1@huawei.com>

auth/pake/pake.c

@@ -313,7 +313,12 @@ CRYPT_EAL_KdfCtx* HITLS_AUTH_PakeGetKdfCtx(HITLS_AUTH_PakeCtx* ctx, HITLS_AUTH_P
 
     switch (kdf.algId) {
         case CRYPT_KDF_PBKDF2: {
-            uint32_t totalLen = ctx->password.dataLen + ctx->prover.dataLen + ctx->verifier.dataLen;
+            uint64_t totalLen64 = (uint64_t)ctx->password.dataLen + ctx->prover.dataLen + ctx->verifier.dataLen;
+            if (totalLen64 > UINT32_MAX) {
+                BSL_ERR_PUSH_ERROR(HITLS_AUTH_INVALID_ARG);
+                return NULL;
+            }
+            uint32_t totalLen = (uint32_t)totalLen64;
             uint8_t *buffer = BSL_SAL_Malloc(totalLen);
             if (buffer == NULL) {
                 BSL_ERR_PUSH_ERROR(HITLS_AUTH_PAKE_MEMORY_ALLOC_FAIL);

auth/pake/spake2plus/src/spake2plus.c

@@ -887,12 +887,12 @@ static void uint32_to_le_bytes(uint32_t len, uint8_t out[8])
     int32_t __ret = HITLS_AUTH_SUCCESS; \
     uint8_t len_bytes[8]; \
     uint32_to_le_bytes(len, len_bytes); \
-    if (remaining < 8 + len) { \
+    if (remaining < LITTLE_BYTEORDER_LEN || len > remaining - LITTLE_BYTEORDER_LEN) { \
         __ret = CRYPT_MEM_ALLOC_FAIL; \
     } else { \
-        (void)memcpy_s(pos, 8, len_bytes, 8); \
-        pos += 8; remaining -= 8; \
-        (void)memcpy_s(pos, len, data, len); \
+        memcpy(pos, len_bytes, LITTLE_BYTEORDER_LEN); \
+        pos += LITTLE_BYTEORDER_LEN; remaining -= LITTLE_BYTEORDER_LEN; \
+        memcpy(pos, data, len); \
         pos += len; remaining -= len; \
     } \
     __ret; \
@@ -938,8 +938,15 @@ static int32_t Spake2PlusComputeTranscript(HITLS_AUTH_PakeCtx *ctx, BSL_Buffer s
         goto ERR;
     }
     if (totalSize) {
-        *totalSize = 8 + context.dataLen + 8 + prover.dataLen + 8 + verifier.dataLen + 8 + mLen + 8 + nLen +
-        8 + shareP.dataLen + 8 + shareV.dataLen + 8 + z.dataLen + 8 + v.dataLen + 8 + spakeCtx->w0.dataLen;
+        uint64_t calcSize = LITTLE_BYTEORDER_LEN * 10 + (uint64_t)context.dataLen + (uint64_t)prover.dataLen +
+            (uint64_t)verifier.dataLen + (uint64_t)mLen + (uint64_t)nLen + (uint64_t)shareP.dataLen +
+            (uint64_t)shareV.dataLen + (uint64_t)z.dataLen + (uint64_t)v.dataLen + (uint64_t)spakeCtx->w0.dataLen;
+        if (calcSize > UINT32_MAX) {
+            BSL_ERR_PUSH_ERROR(HITLS_AUTH_INVALID_ARG);
+            ret = HITLS_AUTH_INVALID_ARG;
+            goto ERR;
+        }
+        *totalSize = (uint32_t)calcSize;
     }
     if (tt) {
         ret = APPEND_FIELD(context.data, context.dataLen);

bsl/base64/src/bsl_base64.c

@@ -385,7 +385,9 @@ int32_t BSL_BASE64_EncodeUpdate(BSL_Base64Ctx *ctx, const uint8_t *srcBuf, uint3
     }
     /* By default, the user selects the line feed, considers the terminator,
        and checks whether the length meets the (srcBufLen + ctx->num)/48*65+1 requirement. */
-    if (*dstBufLen < ((srcBufLen + ctx->num) / HITLS_BASE64_CTX_LENGTH * (BASE64_DECODE_BLOCKSIZE + 1) + 1)) {
+    uint64_t inLen = (uint64_t)srcBufLen + ctx->num;
+    uint64_t needLen = inLen / HITLS_BASE64_CTX_LENGTH * (BASE64_DECODE_BLOCKSIZE + 1) + 1;
+    if (needLen > UINT32_MAX || *dstBufLen < (uint32_t)needLen) {
         BSL_ERR_PUSH_ERROR(BSL_BASE64_BUF_NOT_ENOUGH);
         return BSL_BASE64_BUF_NOT_ENOUGH;
     }
@@ -463,7 +465,9 @@ int32_t BSL_BASE64_DecodeUpdate(BSL_Base64Ctx *ctx, const char *srcBuf, const ui
     }
     /* Estimated maximum value. By default, the input srcBuf is without line feed. Each line contains 64 characters.
        Check whether the length meets the (srcBufLen + ctx->num)/64*48 requirement. */
-    if (*dstBufLen < ((srcBufLen + ctx->num) / BASE64_DECODE_BLOCKSIZE * HITLS_BASE64_CTX_LENGTH)) {
+    uint64_t inLen = (uint64_t)srcBufLen + ctx->num;
+    uint64_t needLen = inLen / BASE64_DECODE_BLOCKSIZE * HITLS_BASE64_CTX_LENGTH;
+    if (needLen > UINT32_MAX || *dstBufLen < (uint32_t)needLen) {
         BSL_ERR_PUSH_ERROR(BSL_BASE64_BUF_NOT_ENOUGH);
         return BSL_BASE64_BUF_NOT_ENOUGH;
     }

bsl/params/src/bsl_param_maker.c

@@ -157,6 +157,10 @@ int32_t BSL_PARAM_MAKER_PushValue(BSL_ParamMaker *maker, int32_t key, uint32_t t
             paramMakerDef->allocLen = 0;
             break;
         case BSL_PARAM_TYPE_UTF8_STR:
+            if (len == UINT32_MAX) {
+                ret = BSL_PARAMS_OUT_LIMIT;
+                goto EXIT;
+            }
             paramMakerDef->value = value;
             paramMakerDef->allocLen = len + 1;
             break;
@@ -168,6 +172,10 @@ int32_t BSL_PARAM_MAKER_PushValue(BSL_ParamMaker *maker, int32_t key, uint32_t t
             ret = BSL_PARAMS_INVALID_TYPE;
             goto EXIT;
     }
+    if (paramMakerDef->allocLen > UINT32_MAX - maker->valueLen) {
+        ret = BSL_PARAMS_OUT_LIMIT;
+        goto EXIT;
+    }
     maker->valueLen += paramMakerDef->allocLen;
     ret = BSL_LIST_AddElement(maker->params, paramMakerDef, BSL_LIST_POS_END);
 EXIT:
@@ -195,6 +203,11 @@ int32_t BSL_PARAM_MAKER_DeepPushValue(BSL_ParamMaker *maker, int32_t key, uint32
     uint32_t allocLen = 0;
     switch (type) {
         case BSL_PARAM_TYPE_UTF8_STR:
+            if (len == UINT32_MAX) {
+                ret = BSL_PARAMS_OUT_LIMIT;
+                BSL_ERR_PUSH_ERROR(ret);
+                goto EXIT;
+            }
             allocLen = len + 1;
             break;
         case BSL_PARAM_TYPE_OCTETS:
@@ -205,6 +218,11 @@ int32_t BSL_PARAM_MAKER_DeepPushValue(BSL_ParamMaker *maker, int32_t key, uint32
             BSL_ERR_PUSH_ERROR(ret);
             goto EXIT;
     }
+    if (allocLen > UINT32_MAX - maker->valueLen) {
+        ret = BSL_PARAMS_OUT_LIMIT;
+        BSL_ERR_PUSH_ERROR(ret);
+        goto EXIT;
+    }
     paramMakerDef->value = BSL_SAL_Malloc(len);
     if (paramMakerDef->value == NULL && value != NULL) {
         ret = BSL_MALLOC_FAIL;

bsl/pem/src/bsl_pem.c

@@ -154,8 +154,14 @@ int32_t BSL_PEM_EncodeAsn1ToPem(uint8_t *asn1Encode, uint32_t asn1Len, BSL_PEM_S
             BSL_ERR_PUSH_ERROR(ret);
             break;
         }
-        uint32_t line = (len + PEM_LINE_LEN - 1) / PEM_LINE_LEN;
-        uint32_t sumLen = line + len + headLen + tailLen + 3; // 3: \n + \n +\0
+        uint64_t line = ((uint64_t)len + PEM_LINE_LEN - 1) / PEM_LINE_LEN;
+        uint64_t sumLen64 = line + len + headLen + tailLen + 3; // 3: \n + \n +\0
+        if (sumLen64 > UINT32_MAX) {
+            BSL_ERR_PUSH_ERROR(BSL_BASE64_BUF_NOT_ENOUGH);
+            ret = BSL_BASE64_BUF_NOT_ENOUGH;
+            break;
+        }
+        uint32_t sumLen = (uint32_t)sumLen64;
         res = BSL_SAL_Malloc(sumLen);
         if (res == NULL) {
             BSL_ERR_PUSH_ERROR(BSL_MALLOC_FAIL);

bsl/uio/src/uio_file.c

@@ -115,6 +115,7 @@ static int32_t FileOpen(BSL_UIO *uio, uint32_t flags, const char *filename)
     }
 
     BSL_UIO_SetCtx(uio, (void *)fileHandle);
+    BSL_UIO_SetIsUnderlyingClosedByUio(uio, true);
     uio->init = true;
     return BSL_SUCCESS;
 }

pki/cms/src/hitls_cms_signdata.c

@@ -405,15 +405,26 @@ static int32_t SignedAttrsCheck(HITLS_X509_Attrs *attrs)
     if (attrs == NULL || BSL_LIST_COUNT(attrs->list) == 0) {
         return HITLS_PKI_SUCCESS;
     }
-    int32_t nums = 0;
+    bool hasContentType = false;
+    bool hasMessageDigest = false;
     for (BslListNode *attrNode = BSL_LIST_FirstNode(attrs->list); attrNode != NULL;
         attrNode = BSL_LIST_GetNextNode(attrs->list, attrNode)) {
         HITLS_X509_AttrEntry *node = (HITLS_X509_AttrEntry *)BSL_LIST_GetData(attrNode);
-        if (node->cid == BSL_CID_PKCS9_AT_MESSAGEDIGEST || node->cid == BSL_CID_PKCS9_AT_CONTENTTYPE) {
-            nums++;
+        if (node->cid == BSL_CID_PKCS9_AT_CONTENTTYPE) {
+            if (hasContentType) {
+                BSL_ERR_PUSH_ERROR(HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID);
+                return HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID;
+            }
+            hasContentType = true;
+        } else if (node->cid == BSL_CID_PKCS9_AT_MESSAGEDIGEST) {
+            if (hasMessageDigest) {
+                BSL_ERR_PUSH_ERROR(HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID);
+                return HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID;
+            }
+            hasMessageDigest = true;
         }
     }
-    if (nums != 2) { // 2: the minimum number of required signed attributes.
+    if (!hasContentType || !hasMessageDigest) {
         BSL_ERR_PUSH_ERROR(HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID);
         return HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID;
     }

testcode/sdv/testcase/bsl/base64/test_suite_sdv_base64.c

@@ -1121,3 +1121,32 @@ EXIT:
     BSL_BASE64_CtxFree(ctx);
 }
 /* END_CASE */
+
+/* BEGIN_CASE */
+void SDV_BSL_BASE64_FUNC_TC016(void)
+{
+    const uint8_t src = 0;
+    const char decSrc[] = "A";
+    char enc[1] = {0};
+    uint8_t dec[1] = {0};
+    uint32_t len = UINT32_MAX;
+    BSL_Base64Ctx *ctx = BSL_BASE64_CtxNew();
+    ASSERT_TRUE(ctx != NULL);
+
+    ASSERT_EQ(BSL_BASE64_EncodeInit(ctx), BSL_SUCCESS);
+    ASSERT_EQ(BSL_BASE64_EncodeUpdate(ctx, &src, UINT32_MAX, enc, &len), BSL_BASE64_BUF_NOT_ENOUGH);
+    ASSERT_EQ(enc[0], 0);
+    TestErrClear();
+
+    ASSERT_EQ(BSL_BASE64_DecodeInit(ctx), BSL_SUCCESS);
+    len = 0;
+    ASSERT_EQ(BSL_BASE64_DecodeUpdate(ctx, decSrc, sizeof(decSrc) - 1, dec, &len), BSL_SUCCESS);
+    ASSERT_EQ(len, 0);
+    len = 0;
+    ASSERT_EQ(BSL_BASE64_DecodeUpdate(ctx, decSrc, UINT32_MAX, dec, &len), BSL_BASE64_BUF_NOT_ENOUGH);
+    ASSERT_EQ(len, 0);
+
+EXIT:
+    BSL_BASE64_CtxFree(ctx);
+}
+/* END_CASE */

testcode/sdv/testcase/bsl/base64/test_suite_sdv_base64.data

@@ -72,3 +72,6 @@ SDV_BSL_BASE64_FUNC_TC014:
 
 SDV_BSL_BASE64_FUNC_TC015
 SDV_BSL_BASE64_FUNC_TC015:
+
+SDV_BSL_BASE64_FUNC_TC016
+SDV_BSL_BASE64_FUNC_TC016:

testcode/sdv/testcase/bsl/params/test_suite_sdv_params_maker.c

@@ -65,6 +65,29 @@ EXIT:
 }
 /* END_CASE */
 
+/* BEGIN_CASE */
+void SDV_BSL_BSL_PARAM_MAKER_Push_Value_Overflow_TC001()
+{
+    uint8_t value = 0;
+    int32_t key = 1;
+    BSL_ParamMaker *maker = BSL_PARAM_MAKER_New();
+    ASSERT_TRUE(maker != NULL);
+
+    ASSERT_EQ(BSL_PARAM_MAKER_PushValue(maker, key++, BSL_PARAM_TYPE_UTF8_STR, &value, UINT32_MAX),
+        BSL_PARAMS_OUT_LIMIT);
+    ASSERT_EQ(BSL_PARAM_MAKER_DeepPushValue(maker, key++, BSL_PARAM_TYPE_UTF8_STR, &value, UINT32_MAX),
+        BSL_PARAMS_OUT_LIMIT);
+    TestErrClear();
+
+    ASSERT_EQ(BSL_PARAM_MAKER_PushValue(maker, key++, BSL_PARAM_TYPE_OCTETS, &value, UINT32_MAX), BSL_SUCCESS);
+    ASSERT_EQ(BSL_PARAM_MAKER_PushValue(maker, key++, BSL_PARAM_TYPE_OCTETS, &value, 1), BSL_PARAMS_OUT_LIMIT);
+
+EXIT:
+    BSL_PARAM_MAKER_Free(maker);
+    return;
+}
+/* END_CASE */
+
 /* BEGIN_CASE */
 void SDV_BSL_BSL_PARAM_MAKER_ToParam_API_TC001()
 {

testcode/sdv/testcase/bsl/params/test_suite_sdv_params_maker.data

@@ -4,6 +4,9 @@ SDV_BSL_BSL_PARAM_MAKER_New_API_TC001:
 SDV_BSL_BSL_PARAM_MAKER_Push_Value_API_TC001
 SDV_BSL_BSL_PARAM_MAKER_Push_Value_API_TC001:
 
+SDV_BSL_BSL_PARAM_MAKER_Push_Value_Overflow_TC001
+SDV_BSL_BSL_PARAM_MAKER_Push_Value_Overflow_TC001:
+
 SDV_BSL_BSL_PARAM_MAKER_ToParam_API_TC001
 SDV_BSL_BSL_PARAM_MAKER_ToParam_API_TC001:
 

testcode/sdv/testcase/bsl/pem/test_suite_sdv_pem.c

@@ -20,12 +20,16 @@
 #include <string.h>
 #include "securec.h"
 #include "bsl_errno.h"
+#include "bsl_base64.h"
 #include "bsl_sal.h"
 #include "sal_file.h"
 #include "bsl_pem_internal.h"
+#include "stub_utils.h"
 
 /* END_HEADER */
 
+STUB_DEFINE_RET4(int32_t, BSL_BASE64_Encode, const uint8_t *, const uint32_t, char *, uint32_t *);
+
 /**
  * @test SDV_BSL_PEM_ISPEM_FUNC_TC001
  * @spec  -
@@ -174,6 +178,52 @@ EXIT:
 }
 /* END_CASE */
 
+static int32_t STUB_BSL_BASE64_Encode_LargeOutput(const uint8_t *srcBuf, const uint32_t srcBufLen,
+    char *dstBuf, uint32_t *dstBufLen)
+{
+    (void)srcBuf;
+    (void)srcBufLen;
+    (void)dstBuf;
+    *dstBufLen = UINT32_MAX - 1;
+    return BSL_SUCCESS;
+}
+
+/**
+ * @test SDV_BSL_PEM_ENCODE_ASN1_OVERFLOW_TC001
+ * @spec  -
+ * @title  ASN.1 to PEM encoding length overflow checks
+ * @precon  nan
+ * @brief   1. Call BSL_PEM_EncodeAsn1ToPem with an ASN.1 length whose Base64 output exceeds uint32_t.
+ *          2. Stub BSL_BASE64_Encode to report a near-uint32_t output length and encode a small ASN.1 buffer.
+ * @expect  1. The encoder rejects the input before allocation.
+ *          2. The PEM formatted length overflow is rejected.
+ * @prior  Level 1
+ * @auto  TRUE
+ */
+/* BEGIN_CASE */
+void SDV_BSL_PEM_ENCODE_ASN1_OVERFLOW_TC001(void)
+{
+    uint8_t asn1 = 0;
+    char *encode = NULL;
+    uint32_t encodeLen = 0;
+    BSL_PEM_Symbol sym = {BSL_PEM_CERT_BEGIN_STR, BSL_PEM_CERT_END_STR};
+
+    ASSERT_EQ(BSL_PEM_EncodeAsn1ToPem(&asn1, 0xC0000000U, &sym, &encode, &encodeLen), BSL_INVALID_ARG);
+    ASSERT_TRUE(encode == NULL);
+    TestErrClear();
+
+    STUB_REPLACE(BSL_BASE64_Encode, STUB_BSL_BASE64_Encode_LargeOutput);
+    ASSERT_EQ(BSL_PEM_EncodeAsn1ToPem(&asn1, sizeof(asn1), &sym, &encode, &encodeLen),
+        BSL_BASE64_BUF_NOT_ENOUGH);
+    ASSERT_TRUE(encode == NULL);
+
+EXIT:
+    STUB_RESTORE(BSL_BASE64_Encode);
+    BSL_SAL_Free(encode);
+    return;
+}
+/* END_CASE */
+
 /**
  * @test SDV_BSL_PEM_BINARY_INPUT_TC001
  * @spec  -

testcode/sdv/testcase/bsl/pem/test_suite_sdv_pem.data

@@ -51,6 +51,10 @@ SDV_BSL_PEM_PARSE_FUNC_TC002:
 
 SDV_BSL_PEM_PARSE_FUNC_TC003
 SDV_BSL_PEM_PARSE_FUNC_TC003:
+
+SDV_BSL_PEM_ENCODE_ASN1_OVERFLOW_TC001
+SDV_BSL_PEM_ENCODE_ASN1_OVERFLOW_TC001:
+
 SDV_BSL_PEM_BINARY_INPUT_TC001
 SDV_BSL_PEM_BINARY_INPUT_TC001:
 

testcode/sdv/testcase/pki/cms/test_suite_sdv_cms_sign.c

@@ -162,6 +162,32 @@ EXIT:
 }
 /* END_CASE */
 
+/**
+ * @test   SDV_CMS_PARSE_SIGNEDDATA_SIGNEDATTRS_INVALID_TC001
+ * @title  Parse CMS SignedData with invalid signedAttrs
+ * @brief
+ *    1. Parse CMS SignedData with repeated required signedAttrs
+ *    2. Parse CMS SignedData with multi-value signedAttrs SET
+ * @expect
+ *    1-2. Returns HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID
+ */
+/* BEGIN_CASE */
+void SDV_CMS_PARSE_SIGNEDDATA_SIGNEDATTRS_INVALID_TC001(char *p7path)
+{
+#if !defined(HITLS_PKI_CMS_SIGNEDDATA)
+    (void)p7path;
+    SKIP_TEST();
+#else
+    HITLS_CMS *cms = NULL;
+    ASSERT_EQ(HITLS_CMS_ProviderParseFile(NULL, NULL, NULL, p7path, &cms),
+        HITLS_CMS_ERR_SIGNEDDATA_SIGNEDATTRS_INVALID);
+EXIT:
+    HITLS_CMS_Free(cms);
+    return;
+#endif
+}
+/* END_CASE */
+
 /**
  * @test   SDV_CMS_PARSE_SIGNEDDATA_ENC_DEC_FILE_TC001
  * @title  Parse, encode and verify CMS SignedData from file

testcode/sdv/testcase/pki/cms/test_suite_sdv_cms_sign.data

@@ -88,6 +88,9 @@ SDV_CMS_PARSE_SIGNEDDATA_VERIFY_TEST_TC001:"../testdata/cert/asn1/cms/signeddata
 SDV_CMS_PARSE_SIGNEDDATA_VERIFY_TEST_TC001 version3 detached signerinfo
 SDV_CMS_PARSE_SIGNEDDATA_VERIFY_TEST_TC001:"../testdata/cert/asn1/cms/signeddata/v3detached.cms":"../testdata/cert/asn1/cms/signeddata/msg.txt":1:"../testdata/cert/asn1/cms/signeddata/noattr/ca_cert.pem":1
 
+SDV_CMS_PARSE_SIGNEDDATA_SIGNEDATTRS_INVALID_TC001 duplicate contentType and missing messageDigest
+SDV_CMS_PARSE_SIGNEDDATA_SIGNEDATTRS_INVALID_TC001:"../testdata/cert/asn1/cms/signeddata/p256_signedattrs_dup_contenttype.cms"
+
 SDV_CMS_PARSE_SIGNEDDATA_ENC_DEC_FILE_TC001 p256 with atttach
 SDV_CMS_PARSE_SIGNEDDATA_ENC_DEC_FILE_TC001:"../testdata/cert/asn1/cms/signeddata/p256_attached.cms":"../testdata/cert/asn1/cms/signeddata/msg.txt":0:"../testdata/cert/asn1/cms/signeddata/ca_cert.pem"