Commit fed15f076f for openssl.org
commit fed15f076fb22676208f70ec21b788589defd071
Author: Daniel Kubec <kubec@openssl.org>
Date: Wed Dec 10 13:57:40 2025 +0100
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 efa2ade30f..aff286a563 100644
--- 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 d2bbc1d9b8..0d81ac69db 100644
--- 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 f0b6c6ad5e..b79ebd6914 100644
--- 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 3af2ddd53d..d75e14983e 100644
--- 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 ec99024716..8482e6b47d 100644
--- 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;
}