ASN1: Reject negative BIGNUM components

In the ASN.1 structures we define the BIGNUM as positive and enforce
this during parsing. If the encoded value is negative, we raise an error
and reject the material.

Fixes #29210
Fixes #27407

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/29370)

crypto/asn1/x_bignum.c

@@ -114,13 +114,21 @@ static int bn_i2c(const ASN1_VALUE **pval, unsigned char *cont, int *putype,
 static int bn_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
     int utype, char *free_cont, const ASN1_ITEM *it)
 {
+    int allocated = 0;
     BIGNUM *bn;
 
-    if (*pval == NULL && !bn_new(pval, it))
+    /* Reject encodings that imply a negative number. */
+    if (len == 0 || (*cont & 0x80) != 0) {
+        ERR_raise(ERR_LIB_ASN1, ASN1_R_INVALID_VALUE);
+        return 0;
+    }
+
+    if (*pval == NULL && (allocated = bn_new(pval, it)) == 0)
         return 0;
     bn = (BIGNUM *)*pval;
     if (!BN_bin2bn(cont, len, bn)) {
-        bn_free(pval, it);
+        if (allocated != 0)
+            bn_free(pval, it);
         return 0;
     }
     return 1;
@@ -129,15 +137,19 @@ static int bn_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
 static int bn_secure_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
     int utype, char *free_cont, const ASN1_ITEM *it)
 {
-    int ret;
+    int ret, allocated = 0;
     BIGNUM *bn;
 
-    if (*pval == NULL && !bn_secure_new(pval, it))
+    if (*pval == NULL && (allocated = bn_secure_new(pval, it)) == 0)
         return 0;
 
     ret = bn_c2i(pval, cont, len, utype, free_cont, it);
-    if (!ret)
+    if (!ret) {
+        if (allocated != 0)
+            bn_free(pval, it);
+
         return 0;
+    }
 
     /* Set constant-time flag for all secure BIGNUMS */
     bn = (BIGNUM *)*pval;

test/crltest.c

@@ -638,27 +638,6 @@ static X509_CRL *crl_clear_err_parse(const char **pem)
     return CRL_from_strings(pem);
 }
 
-/*
- * Checks whether a specific error reason is present in the error stack.
- * This function iterates over the current thread's error queue using
- * ERR_get_error_all(), extracting all pending errors. If any of them match
- * the specified reason code (as returned by ERR_GET_REASON()), the function
- * returns 1 to indicate that the corresponding error was found.
- */
-static int err_chk(int lib, int reason)
-{
-#if defined(OPENSSL_NO_ERR) || defined(OPENSSL_SMALL_FOOTPRINT) || defined(OPENSSL_NO_DEPRECATED_3_0) || defined(OPENSSL_NO_HTTP)
-    return 1;
-#endif
-    unsigned long e;
-
-    while ((e = ERR_get_error_all(NULL, NULL, NULL, NULL, NULL)))
-        if (ERR_GET_LIB(e) == lib && ERR_GET_REASON(e) == reason)
-            return 1;
-
-    return 0;
-}
-
 static int test_crl_date_invalid(void)
 {
     X509_CRL *tmm = NULL, *tss = NULL, *utc = NULL;

test/testutil.h

@@ -663,5 +663,13 @@ X509_CRL *CRL_from_strings(const char **pem);
  * into |*out| so we can free it.
  */
 BIO *glue2bio(const char **pem, char **out);
+/*
+ * Checks whether a specific error reason is present in the error stack.
+ * This function iterates over the current thread's error queue using
+ * ERR_get_error_all(), extracting all pending errors. If any of them match
+ * the specified reason code (as returned by ERR_GET_REASON()), the function
+ * returns 1 to indicate that the corresponding error was found.
+ */
+int err_chk(int lib, int reason);
 
 #endif /* OSSL_TESTUTIL_H */

test/testutil/load.c

@@ -157,3 +157,22 @@ X509 *X509_from_strings(const char **pem)
     BIO_free(b);
     return x;
 }
+
+int err_chk(int lib, int reason)
+{
+#if defined(OPENSSL_NO_ERR) || defined(OPENSSL_SMALL_FOOTPRINT)
+    return 1;
+#endif
+#if defined(OPENSSL_NO_DEPRECATED_3_0) || defined(OPENSSL_NO_HTTP)
+    return 1;
+#endif
+
+    unsigned long e;
+
+    while ((e = ERR_get_error_all(NULL, NULL, NULL, NULL, NULL))) {
+        if (ERR_GET_LIB(e) == lib && ERR_GET_REASON(e) == reason)
+            return 1;
+    }
+
+    return 0;
+}

test/x509_internal_test.c

@@ -495,11 +495,43 @@ static int tests_X509_check_time(void)
     return do_x509_time_tests(cert_test_data, sizeof(cert_test_data) / sizeof(CERT_TEST_DATA));
 }
 
+static const char *kRSAModulusNeg[] = {
+    "-----BEGIN CERTIFICATE-----\n",
+    "MIIByjCCAXSgAwIBAgIQBjdsAKoAZIoRz7jUqlw19DANBgkqhkiG9w0BAQQFADAW\n",
+    "MRQwEgYDVQQDEwtSb290IEFnZW5jeTAeFw05NjA1MjgyMjAyNTlaFw0zOTEyMzEy\n",
+    "MzU5NTlaMBYxFDASBgNVBAMTC1Jvb3QgQWdlbmN5MFswDQYJKoZIhvcNAQEBBQAD\n",
+    "SgAwRwJAgVUiuYqkb+3W59lmD1W8183VvE5AAiGisfeHMIVe0vJEudybdbb7Rl9C\n",
+    "tp0jNgveVA/NvR+ZKhBYEctAy7WnQQIDAQABo4GeMIGbMFAGA1UEAwRJE0dGb3Ig\n",
+    "VGVzdGluZyBQdXJwb3NlcyBPbmx5IFNhbXBsZSBTb2Z0d2FyZSBQdWJsaXNoaW5n\n",
+    "IENyZWRlbnRpYWxzIEFnZW5jeTBHBgNVHQEEQDA+gBAS5AktBh0dTwCNYSHcFmRj\n",
+    "oRgwFjEUMBIGA1UEAxMLUm9vdCBBZ2VuY3mCEAY3bACqAGSKEc+41KpcNfQwDQYJ\n",
+    "KoZIhvcNAQEEBQADQQAtLj57iUKJP6ghF/rw9cOV22JpW8ncwbP68MRvb2Savecb\n",
+    "JWhyg2e9VrCNAb0q98xLvYeluocgTEIRQa0QFzuM\n",
+    "-----END CERTIFICATE-----\n",
+    NULL
+};
+
+static int tests_X509_check_crypto(void)
+{
+    X509 *rsa_n_neg = NULL;
+    EVP_PKEY *pub = NULL;
+    int test;
+
+    test = TEST_ptr((rsa_n_neg = X509_from_strings(kRSAModulusNeg)))
+        && TEST_ptr_null((pub = X509_get_pubkey(rsa_n_neg)))
+        && TEST_true(err_chk(ERR_LIB_EVP, EVP_R_DECODE_ERROR));
+
+    EVP_PKEY_free(pub);
+    X509_free(rsa_n_neg);
+    return test;
+}
+
 int setup_tests(void)
 {
     ADD_TEST(test_standard_exts);
     ADD_ALL_TESTS(test_a2i_ipaddress, OSSL_NELEM(a2i_ipaddress_tests));
     ADD_TEST(tests_X509_PURPOSE);
     ADD_TEST(tests_X509_check_time);
+    ADD_TEST(tests_X509_check_crypto);
     return 1;
 }