From 6bba0a8355db632f7287905ec577483979c47905 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2024 23:32:50 +0200 Subject: [PATCH 1/3] Fix stack buffer overflow in ECDSA signature format conversions Signed-off-by: Gilles Peskine --- ChangeLog.d/ecdsa-conversion-overflow.txt | 4 ++++ library/psa_util.c | 6 ++++++ tests/suites/test_suite_psa_crypto_util.data | 6 ++++++ 3 files changed, 16 insertions(+) create mode 100644 ChangeLog.d/ecdsa-conversion-overflow.txt diff --git a/ChangeLog.d/ecdsa-conversion-overflow.txt b/ChangeLog.d/ecdsa-conversion-overflow.txt new file mode 100644 index 0000000000..00cac06513 --- /dev/null +++ b/ChangeLog.d/ecdsa-conversion-overflow.txt @@ -0,0 +1,4 @@ +Security + * Fix a stack buffer overflow in mbedtls_ecdsa_der_to_raw() and + mbedtls_ecdsa_raw_to_der() when curve_bits is larger than the + largest supported curve. diff --git a/library/psa_util.c b/library/psa_util.c index 4ccc5b05d8..679d00ea9b 100644 --- a/library/psa_util.c +++ b/library/psa_util.c @@ -443,6 +443,9 @@ int mbedtls_ecdsa_raw_to_der(size_t bits, const unsigned char *raw, size_t raw_l if (raw_len != (2 * coordinate_len)) { return MBEDTLS_ERR_ASN1_INVALID_DATA; } + if (coordinate_len > sizeof(r)) { + return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } /* Since raw and der buffers might overlap, dump r and s before starting * the conversion. */ @@ -561,6 +564,9 @@ int mbedtls_ecdsa_der_to_raw(size_t bits, const unsigned char *der, size_t der_l if (raw_size < coordinate_size * 2) { return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; } + if (2 * coordinate_size > sizeof(raw_tmp)) { + return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } /* Check that the provided input DER buffer has the right header. */ ret = mbedtls_asn1_get_tag(&p, der + der_len, &data_len, diff --git a/tests/suites/test_suite_psa_crypto_util.data b/tests/suites/test_suite_psa_crypto_util.data index 807007b5e6..34e5065d33 100644 --- a/tests/suites/test_suite_psa_crypto_util.data +++ b/tests/suites/test_suite_psa_crypto_util.data @@ -6,6 +6,9 @@ ECDSA Raw -> DER, 256bit, DER buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_raw_to_der:256:"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222":"304402201111111111111111111111111111111111111111111111111111111111111111022022222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +ECDSA Raw -> DER, very large input (544-bit) +ecdsa_raw_to_der:544:"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"deadbeef":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + ECDSA Raw -> DER, 256bit, Null r depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_raw_to_der:256:"00000000000000000000000000000000000000000000000000000000000000002222222222222222222222222222222222222222222222222222222222222222":"30440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_INVALID_DATA @@ -58,6 +61,9 @@ ECDSA DER -> Raw, 256bit, Raw buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_der_to_raw:256:"30440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":"111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +ECDSA DER -> Raw, very large input (544-bit) +ecdsa_der_to_raw:544:"30818c0244111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111102442222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + ECDSA DER -> Raw, 256bit, Wrong sequence tag depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_der_to_raw:256:"40440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG From db81d7efb0482a661722d9392ec11c1fabea5479 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jun 2024 10:46:25 +0200 Subject: [PATCH 2/3] More diversified sizes in tests Test the minimum size that caused an overflow in all configurations, and also a mostly arbitrary larger size. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto_util.data | 22 ++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_util.data b/tests/suites/test_suite_psa_crypto_util.data index 34e5065d33..c84a8368cd 100644 --- a/tests/suites/test_suite_psa_crypto_util.data +++ b/tests/suites/test_suite_psa_crypto_util.data @@ -6,8 +6,15 @@ ECDSA Raw -> DER, 256bit, DER buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_raw_to_der:256:"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222":"304402201111111111111111111111111111111111111111111111111111111111111111022022222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL -ECDSA Raw -> DER, very large input (544-bit) -ecdsa_raw_to_der:544:"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"deadbeef":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +# Check coordinates one byte larger than the largest supported curve. +# If we add an even larger curve, this test case will fail in the full +# configuration because mbedtls_ecdsa_raw_to_der() will return 0, and we'll +# need to use larger data for this test case. +ECDSA Raw -> DER, very large input (536-bit) +ecdsa_raw_to_der:536:"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"30818a024311111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111024322222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + +ECDSA Raw -> DER, very large input (1016-bit) +ecdsa_raw_to_der:1016:"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"30820102027f11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111027f22222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ECDSA Raw -> DER, 256bit, Null r depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 @@ -61,8 +68,15 @@ ECDSA DER -> Raw, 256bit, Raw buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_der_to_raw:256:"30440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":"111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL -ECDSA DER -> Raw, very large input (544-bit) -ecdsa_der_to_raw:544:"30818c0244111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111102442222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +# Check coordinates one byte larger than the largest supported curve. +# If we add an even larger curve, this test case will fail in the full +# configuration because mbedtls_ecdsa_der_to_raw() will return 0, and we'll +# need to use larger data for this test case. +ECDSA DER -> Raw, very large input (536-bit) +ecdsa_der_to_raw:536:"30818a024311111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111024322222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + +ECDSA DER -> Raw, very large input (1016-bit) +ecdsa_der_to_raw:1016:"30820102027f11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111027f22222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ECDSA DER -> Raw, 256bit, Wrong sequence tag depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 From a9e7ac9811054b14c3e754a2735b7ff70e0cf8ad Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jun 2024 10:59:55 +0200 Subject: [PATCH 3/3] Improve description of who is affected Signed-off-by: Gilles Peskine --- ChangeLog.d/ecdsa-conversion-overflow.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/ecdsa-conversion-overflow.txt b/ChangeLog.d/ecdsa-conversion-overflow.txt index 00cac06513..83b7f2f88b 100644 --- a/ChangeLog.d/ecdsa-conversion-overflow.txt +++ b/ChangeLog.d/ecdsa-conversion-overflow.txt @@ -1,4 +1,6 @@ Security * Fix a stack buffer overflow in mbedtls_ecdsa_der_to_raw() and - mbedtls_ecdsa_raw_to_der() when curve_bits is larger than the - largest supported curve. + mbedtls_ecdsa_raw_to_der() when the bits parameter is larger than the + largest supported curve. In some configurations with PSA disabled, + all values of bits are affected. This never happens in internal library + calls, but can affect applications that call these functions directly.