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)

diff --git a/crypto/asn1/x_bignum.c b/crypto/asn1/x_bignum.c
index efa2ade30fad3210c5056fa74c1ef58ff6881f93..aff286a563a366fae0ed4007794fff799cbfbb3e 100644 (file)
--- a/crypto/asn1/x_bignum.c
+++ b/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;

diff --git a/test/crltest.c b/test/crltest.c
index d2bbc1d9b8a7e99c4ae0f86dea6e3ab98c2b4f8a..0d81ac69dbf786a9b07029d4f4fc53725b652b85 100644 (file)
--- a/test/crltest.c
+++ b/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;

diff --git a/test/testutil.h b/test/testutil.h
index f0b6c6ad5ea0d56a5b7435ea9581002789767573..b79ebd6914a1669ce96966c03c109e55a58a10d4 100644 (file)
--- a/test/testutil.h
+++ b/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 */

diff --git a/test/testutil/load.c b/test/testutil/load.c
index 3af2ddd53d8c41ec003aceea13e3cd665ce582ea..d75e14983e39109257eabd8987922c3e2e35d1c0 100644 (file)
--- a/test/testutil/load.c
+++ b/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;
+}

diff --git a/test/x509_internal_test.c b/test/x509_internal_test.c
index ec9902471679f6fc2c60816f97a4b8934d03c8ce..8482e6b47d4bd26b722f935503b897cf5843e495 100644 (file)
--- a/test/x509_internal_test.c
+++ b/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;
 }