Replaced casts with safety macros; improved OpenSSL X509 errors repor…

apps/unit_tests/cast_helpers_unit_tests.c

@@ -58,6 +58,12 @@ castTestUlongToInt(unsigned long src, int* dst) {
     return(0);
 }
 
+static int
+castTestUlongLongToInt(unsigned long long src, int* dst) {
+    XMLSEC_SAFE_CAST_ULLONG_TO_INT(src, (*dst), return(-1), NULL);
+    return(0);
+}
+
 static int
 castTestLongToInt(long src, int* dst) {
     XMLSEC_SAFE_CAST_LONG_TO_INT(src, (*dst), return(-1), NULL);
@@ -366,6 +372,44 @@ test_safe_cast_ulong_to_int(void) {
     testFinishedFailure();
 }
 
+static void
+test_safe_cast_ulonglong_to_int(void) {
+    int dst = 0;
+    int ret;
+
+    testStart("XMLSEC_SAFE_CAST_ULLONG_TO_INT");
+
+    /* valid values: min and max */
+    ret = castTestUlongLongToInt(0, &dst);
+    if((ret != 0) || (dst != 0)) {
+        testLog("Error: valid value 0 was rejected or mis-cast\n");
+        goto failed;
+    }
+    ret = castTestUlongLongToInt((unsigned long long)INT_MAX, &dst);
+    if((ret != 0) || (dst != INT_MAX)) {
+        testLog("Error: valid value INT_MAX was rejected or mis-cast\n");
+        goto failed;
+    }
+
+    /* out-of-range values must be rejected and leave dst unmodified (only
+     * testable when unsigned long long is wider than int so that the value
+     * is representable) */
+#if (ULLONG_MAX > INT_MAX)
+    dst = 0;
+    ret = castTestUlongLongToInt(((unsigned long long)INT_MAX) + 1ULL, &dst);
+    if((ret == 0) || (dst != 0)) {
+        testLog("Error: value INT_MAX+1 (above the destination max) was not rejected\n");
+        goto failed;
+    }
+#endif /* (ULLONG_MAX > INT_MAX) */
+
+    testFinishedSuccess();
+    return;
+
+failed:
+    testFinishedFailure();
+}
+
 static void
 test_safe_cast_long_to_int(void) {
 #if (LONG_MIN < INT_MIN) || (LONG_MAX > INT_MAX)
@@ -1200,6 +1244,7 @@ test_cast_helpers(void) {
     /* to int */
     test_safe_cast_uint_to_int();
     test_safe_cast_ulong_to_int();
+    test_safe_cast_ulonglong_to_int();
     test_safe_cast_long_to_int();
     test_safe_cast_size_t_to_int();
     test_safe_cast_size_to_int();

src/bn.c

@@ -457,7 +457,7 @@ xmlSecBnDiv(xmlSecBnPtr bn, int divider, int* mod) {
         data[ii] = (xmlSecByte)(over / dividerULL);
         over     = over % dividerULL;
     }
-    (*mod) = (int)over;
+    XMLSEC_SAFE_CAST_ULLONG_TO_INT(over, (*mod), return(-1), NULL);
 
     /* remove leading zeros */
     for(ii = 0; ii < size; ii++) {

src/cast_helpers.h

@@ -106,6 +106,23 @@
         int, (dstVal), "%d", INT_MIN, INT_MAX,                                 \
         errorAction, (errorObject))
 
+/* Safe cast with limits check: unsigned long long -> int (unsigned long long is non-negative, so only the max is checked) */
+#if (ULLONG_MAX > INT_MAX)
+
+#define XMLSEC_SAFE_CAST_ULLONG_TO_INT(srcVal, dstVal, errorAction, errorObject) \
+    XMLSEC_SAFE_CAST_MAX_CHECK(unsigned long long, (srcVal), "%llu",             \
+        int, (dstVal), "%d", INT_MIN, INT_MAX,                                   \
+        errorAction, (errorObject))
+
+#else /* (ULLONG_MAX > INT_MAX) */
+
+#define XMLSEC_SAFE_CAST_ULLONG_TO_INT(srcVal, dstVal, errorAction, errorObject) \
+    do {                                                                        \
+        (dstVal) = (srcVal); /* errorAction/errorObject unused: cast always fits */ \
+    } while(0)
+
+#endif /* (ULLONG_MAX > INT_MAX) */
+
 /* Safe cast with limits check: long -> int (checks both min and max) */
 #define XMLSEC_SAFE_CAST_LONG_TO_INT(srcVal, dstVal, errorAction, errorObject) \
     XMLSEC_SAFE_CAST_MIN_MAX_CHECK(long, (srcVal), "%ld",                      \

src/gnutls/x509vfy.c

@@ -338,15 +338,16 @@ xmlSecGnuTLSX509StoreGetCrls(
             return(-1);
         }
 
-        /* verify caller-supplied crl (time + signature); drop it if it fails */
+        /* verify caller-supplied crl (time + signature); fail closed if it fails */
         ret = xmlSecGnuTLSX509StoreVerifyCrlInternal(store, crl, extra_certs, keyInfoCtx);
         if(ret < 0) {
             xmlSecInternalError("xmlSecGnuTLSX509StoreVerifyCrlInternal", NULL);
             xmlFree(res);
             return(-1);
         } else if(ret != 1) {
-            /* crl failed verification, drop it */
-            continue;
+            /* crl failed verification: this is a hard failure because we expect CRLs to be valid */
+            xmlFree(res);
+            return(-1);
         }
         res[res_pos] = crl;
         ++res_pos;

src/mscng/app.c

@@ -70,9 +70,11 @@ xmlSecMSCngAppParseConfig(const char* config, LPTSTR* pCurrentUserStoreName, LPT
         }
     } else {
         /* two-part format: <current-user>:<local-machine> */
-        size_t currentUserLen = (size_t)(colonPos - config);
+        xmlSecSize currentUserLen;
         const char* localMachinePart = colonPos + 1;
 
+        XMLSEC_SAFE_CAST_PTRDIFF_TO_SIZE((colonPos - config), currentUserLen, return(-1), NULL);
+
         if(currentUserLen > 0) {
             char* tmp = (char*)xmlMalloc(currentUserLen + 1);
             if(tmp == NULL) {
@@ -284,7 +286,7 @@ xmlSecMSCngAppKeyLoadEx(const char *filename, xmlSecKeyDataType type, xmlSecKeyD
         XMLSEC_SAFE_CAST_SIZE_TO_ULONG(bufSize, dwDataSize, {xmlSecBufferFinalize(&buffer); return(NULL);}, NULL);
 
         /* Try to read private key first and if no luck, try public key
-         * 
+         *
          * Note: xmlSecMSCngAppKeyReadPrivKeyFromDer() only supports DH and X25519 PKCS#8
          * private keys; other private key types (RSA/EC/DSA) are not supported in DER form
          * by this backend. Public-key DER files are handled by xmlSecMSCngAppKeyReadPubKeyFromDer(). */

src/mscng/x509vfy.c

@@ -558,7 +558,7 @@ xmlSecMSCngX509StoreContainsCert(HCERTSTORE store, CERT_NAME_BLOB* name, PCCERT_
 
     while (TRUE) {
         /* storeCert will be released in the next CertFindCertificateInStore() call
-         * (see https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore) */        
+         * (see https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore) */
         storeCert = CertFindCertificateInStore(store,
             X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
             0,
@@ -708,7 +708,7 @@ struct xmlSecMSCngX509StoreVerifyCertificateChainStep {
     BOOL freeCert;
 };
 #define XMLSEC_MSCNG_X509_STORE_VERIFY_CERTIFICATE_CHAIN_STEP_SIZE 32
-#define XMLSEC_MSCNG_X509_STORE_VERIFY_CERTIFICATE_CHAIN_MAX_DEPTH 1000
+#define XMLSEC_MSCNG_X509_STORE_VERIFY_CERTIFICATE_CHAIN_MAX_DEPTH 100
 #define XMLSEC_MSCNG_X509_CERT_HASH_SIZE 20
 
 /* Returns the SHA1 hash of @p pCert in @p pHash. Returns 0 on success, -1 on error. */

src/mscrypto/x509vfy.c

@@ -431,7 +431,7 @@ struct xmlSecMSCryptoBuildCertChainStep {
     BOOL freeCert;
 };
 #define XMLSEC_MSCRYPTO_BUILD_CERT_CHAIN_STEP_SIZE 32
-#define XMLSEC_MSCRYPTO_BUILD_CERT_CHAIN_MAX_DEPTH 1000
+#define XMLSEC_MSCRYPTO_BUILD_CERT_CHAIN_MAX_DEPTH 100
 #define XMLSEC_MSCRYPTO_X509_CERT_HASH_SIZE 20
 
 /* Returns the SHA1 hash of @p pCert in @p pHash. Returns 0 on success, -1 on error. */

src/nss/app.c

@@ -194,16 +194,11 @@ xmlSecNssAppReadSECItem(SECItem *contents, const char *fn) {
         goto done;
     }
     /*
-     * info.size is PROffset32 (int) in this NSPR; ensure it is non-negative
-     * before casting to unsigned int (the limit for a single PR_Read call and
-     * for the SECItem.len field).
+     * info.size is PROffset32 (int) in this NSPR; check that it fits into
+     * unsigned int (the limit for a single PR_Read call and for the
+     * SECItem.len field).
      */
-    if (info.size < 0) {
-        xmlSecNssError2("PR_GetOpenFileInfo", NULL,
-                        "filename=%s", xmlSecErrorsSafeString(fn));
-        goto done;
-    }
-    ulen = (unsigned int)info.size;
+    XMLSEC_SAFE_CAST_INT_TO_UINT(info.size, ulen, goto done, NULL);
 
     contents->data = 0;
     if (!SECITEM_AllocItem(NULL, contents, ulen)) {

src/openssl/private.h

@@ -97,8 +97,11 @@ X509*           xmlSecOpenSSLX509CertLoadBIO                    (BIO* bio,
 X509_CRL*       xmlSecOpenSSLX509CrlLoadBIO                     (BIO* bio,
                                                                  xmlSecKeyDataFormat format);
 
-int             xmlSecOpenSSLX509Asn1TimeToTime                 (const ASN1_TIME * t, time_t * res);
-void            xmlSecOpenSSLX509NameToString                   (XMLSEC_OPENSSL400_CONST X509_NAME* name, char* buf, int bufLen);
+int             xmlSecOpenSSLX509Asn1TimeToTime                 (const ASN1_TIME * t,
+                                                                 time_t * res);
+int             xmlSecOpenSSLX509NameToString                   (XMLSEC_OPENSSL400_CONST X509_NAME* name,
+                                                                 char* buf,
+                                                                 int bufLen);
 
 
 STACK_OF(X509)*        xmlSecOpenSSLKeyDataX509GetCerts         (xmlSecKeyDataPtr data);

src/openssl/signatures_legacy.c

@@ -707,7 +707,7 @@ xmlSecOpenSSLSignatureLegacyEcdsa_OpenSSLToXmlDsig(
             xmlSecOpenSSLError("i2d_ECDSA_SIG", NULL);
             return(-1);
         }
-        outSize = (xmlSecSize)ret;
+        XMLSEC_SAFE_CAST_INT_TO_SIZE(ret, outSize, return(-1), NULL);
 
         ret = xmlSecBufferSetData(out, outData, outSize);
         if(ret < 0) {

src/openssl/x509.c

@@ -1877,47 +1877,77 @@ xmlSecOpenSSLX509CrlDerRead(const xmlSecByte* buf, xmlSecSize size) {
     return(res);
 }
 
-void
+int
 xmlSecOpenSSLX509NameToString(XMLSEC_OPENSSL400_CONST X509_NAME* name, char* buf, int bufLen) {
-    BIO* mem;
+    BIO* mem = NULL;
     char* data = NULL;
     long len;
+    size_t lenSize;
+    int res = -1;
+
+    xmlSecAssert2(buf != NULL, -1);
+    xmlSecAssert2(bufLen > 0, -1);
 
-    if(buf == NULL || bufLen <= 0) {
-        return;
-    }
     buf[0] = '\0';
     if(name == NULL) {
-        return;
+        /* no name: leave buf empty */
+        return(0);
     }
+
     mem = BIO_new(BIO_s_mem());
     if(mem == NULL) {
-        return;
+        xmlSecOpenSSLError("BIO_new", NULL);
+        goto done;
+    }
+    if(X509_NAME_print_ex(mem, name, 0, XN_FLAG_RFC2253) <= 0) {
+        xmlSecOpenSSLError("X509_NAME_print_ex", NULL);
+        goto done;
     }
-    X509_NAME_print_ex(mem, name, 0, XN_FLAG_RFC2253);
     len = BIO_get_mem_data(mem, &data);
-    if(data != NULL && len > 0) {
-        if(len > bufLen - 1) {
-            len = bufLen - 1;
-        }
-        memcpy(buf, data, (size_t)len);
-        buf[len] = '\0';
+    if((data == NULL) || (len <= 0)) {
+        xmlSecOpenSSLError("BIO_get_mem_data", NULL);
+        goto done;
+    }
+
+    /* truncate the name if needed */
+    if(len > bufLen - 1) {
+        len = bufLen - 1;
     }
-    BIO_free(mem);
+    XMLSEC_SAFE_CAST_LONG_TO_SIZE(len, lenSize, goto done, NULL);
+    memcpy(buf, data, lenSize);
+    buf[len] = '\0';
+
+    /* success */
+    res = 0;
+
+done:
+    if(mem != NULL) {
+        BIO_free(mem);
+    }
+    return(res);
 }
 
 static void
 xmlSecOpenSSLX509CertDebugDump(X509* cert, FILE* output) {
     char buf[1024];
     BIGNUM *bn = NULL;
+    int ret;
 
     xmlSecAssert(cert != NULL);
     xmlSecAssert(output != NULL);
 
-    xmlSecOpenSSLX509NameToString(X509_get_subject_name(cert), buf, sizeof(buf));
-    fprintf(output, "==== Subject Name: %s\n", buf);
-    xmlSecOpenSSLX509NameToString(X509_get_issuer_name(cert), buf, sizeof(buf));
-    fprintf(output, "==== Issuer Name: %s\n", buf);
+    ret = xmlSecOpenSSLX509NameToString(X509_get_subject_name(cert), buf, sizeof(buf));
+    if(ret >= 0) {
+        fprintf(output, "==== Subject Name: %s\n", buf);
+    } else {
+        fprintf(output, "==== Subject Name: unknown\n");
+    }
+    ret = xmlSecOpenSSLX509NameToString(X509_get_issuer_name(cert), buf, sizeof(buf));
+    if(ret >= 0) {
+        fprintf(output, "==== Issuer Name: %s\n", buf);
+    } else {
+        fprintf(output, "==== Issuer Name: unknown\n");
+    }
     fprintf(output, "==== Issuer Serial: ");
     bn = ASN1_INTEGER_to_BN(X509_get_serialNumber(cert),NULL);
     if(bn != NULL) {
@@ -1934,19 +1964,24 @@ static void
 xmlSecOpenSSLX509CertDebugXmlDump(X509* cert, FILE* output) {
     char buf[1024];
     BIGNUM *bn = NULL;
+    int ret;
 
     xmlSecAssert(cert != NULL);
     xmlSecAssert(output != NULL);
 
     fprintf(output, "<SubjectName>");
-    xmlSecOpenSSLX509NameToString(X509_get_subject_name(cert), buf, sizeof(buf));
-    xmlSecPrintXmlString(output, BAD_CAST buf);
+    ret = xmlSecOpenSSLX509NameToString(X509_get_subject_name(cert), buf, sizeof(buf));
+    if(ret >= 0) {
+        xmlSecPrintXmlString(output, BAD_CAST buf);
+    }
     fprintf(output, "</SubjectName>\n");
 
 
     fprintf(output, "<IssuerName>");
-    xmlSecOpenSSLX509NameToString(X509_get_issuer_name(cert), buf, sizeof(buf));
-    xmlSecPrintXmlString(output, BAD_CAST buf);
+    ret = xmlSecOpenSSLX509NameToString(X509_get_issuer_name(cert), buf, sizeof(buf));
+    if(ret >= 0) {
+        xmlSecPrintXmlString(output, BAD_CAST buf);
+    }
     fprintf(output, "</IssuerName>\n");
 
     fprintf(output, "<SerialNumber>");