From fed15f076fb22676208f70ec21b788589defd071 Mon Sep 17 00:00:00 2001 From: Daniel Kubec Date: Wed, 10 Dec 2025 13:57:40 +0100 Subject: [PATCH] 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/29370) --- crypto/asn1/x_bignum.c | 22 +++++++++++++++++----- test/crltest.c | 21 --------------------- test/testutil.h | 8 ++++++++ test/testutil/load.c | 19 +++++++++++++++++++ test/x509_internal_test.c | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 26 deletions(-) diff --git a/crypto/asn1/x_bignum.c b/crypto/asn1/x_bignum.c index efa2ade30fa..aff286a563a 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 d2bbc1d9b8a..0d81ac69dbf 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 f0b6c6ad5ea..b79ebd6914a 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 3af2ddd53d8..d75e14983e3 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 ec990247167..8482e6b47d4 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; } -- 2.47.3