From 920c861cdd83bdee7760b54e052e8f89471fe21d Mon Sep 17 00:00:00 2001 From: yi chen <94xhn1@gmail.com> Date: Sun, 12 Jul 2026 01:53:41 +0800 Subject: [PATCH] Reject zero-length serial number in mbedtls_x509write_crt mbedtls_x509write_crt_set_serial_raw() only checked the upper bound on serial_len (rejecting anything longer than MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) but silently accepted serial_len == 0. That's also the state a freshly-initialized mbedtls_x509write_cert context is left in if the setter is never called at all, since mbedtls_x509write_crt_init() zeroes the whole struct. mbedtls_x509write_crt_der() then encodes ctx->serial/ctx->serial_len directly as an ASN.1 INTEGER: MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_raw_buffer(&c, buf, ctx->serial, ctx->serial_len)); if (*c & 0x80) { ... } else { MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&c, buf, ctx->serial_len)); } MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_tag(&c, buf, MBEDTLS_ASN1_INTEGER)); With serial_len == 0, mbedtls_asn1_write_raw_buffer(..., 0) is a no-op: it decrements nothing and returns 0, leaving the cursor `c` untouched. The following `*c & 0x80` test then reads whatever byte was written immediately before the serial field - in practice the tag byte of the just-written Signature AlgorithmIdentifier SEQUENCE (0x30, high bit clear) - so the `else` branch always runs, writing a length octet of 0x00 followed by the INTEGER tag 0x02. The CertificateSerialNumber field is thus encoded as the two bytes `02 00`. Per X.690 8.3.1, an INTEGER's contents "shall consist of one or more octets"; `02 00` has zero content octets and is not a valid DER encoding at all. Confirmed independently with `openssl asn1parse -inform DER` on the standalone 2 bytes `02 00`, which reports "BAD INTEGER". mbedtls's own parser (mbedtls_x509_get_serial() in library/x509.c) has no length check either and round-trips it silently, but other, stricter X.509/ASN.1 parsers reject certificates built this way. Reject serial_len == 0 in mbedtls_x509write_crt_set_serial_raw(), and add the same check directly in mbedtls_x509write_crt_der() as a defense-in-depth measure for the case where the setter was never called at all. Verified with a standalone host-C program that copies the exact mbedtls_asn1_write_len()/mbedtls_asn1_write_tag()/ mbedtls_asn1_write_raw_buffer() bodies and the vulnerable snippet from mbedtls_x509write_crt_der() verbatim: with serial_len == 0 it deterministically produces `02 00` (confirmed BAD by openssl asn1parse), while serial_len == 1 with content byte 0x00 correctly produces the valid `02 01 00`, and a normal multi-byte serial is unaffected. Signed-off-by: yi chen <94xhn1@gmail.com> --- .../fix-x509write-crt-zero-length-serial.txt | 13 +++++++++++++ include/mbedtls/x509_crt.h | 4 ++-- library/x509write_crt.c | 13 ++++++++++++- tests/suites/test_suite_x509write.function | 7 +++++++ 4 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 ChangeLog.d/fix-x509write-crt-zero-length-serial.txt diff --git a/ChangeLog.d/fix-x509write-crt-zero-length-serial.txt b/ChangeLog.d/fix-x509write-crt-zero-length-serial.txt new file mode 100644 index 0000000000..396ed2e906 --- /dev/null +++ b/ChangeLog.d/fix-x509write-crt-zero-length-serial.txt @@ -0,0 +1,13 @@ +Bugfix + * Fix mbedtls_x509write_crt_set_serial_raw() silently accepting a + zero-length serial number, and mbedtls_x509write_crt_der() silently + encoding it as a syntactically invalid ASN.1 INTEGER (zero content + octets, which X.690 8.3.1 forbids) for the certificate's + CertificateSerialNumber field instead of failing. Certificates + generated this way (either by explicitly passing a zero-length + serial, or by never calling mbedtls_x509write_crt_set_serial_raw() + at all) could be rejected by other, stricter X.509/ASN.1 parsers. + mbedtls_x509write_crt_set_serial_raw() now returns + MBEDTLS_ERR_X509_BAD_INPUT_DATA for a zero-length serial, and + mbedtls_x509write_crt_der() now also rejects a zero-length serial + as a defense-in-depth check. diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 0a7b532404..6d41002e67 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -957,8 +957,8 @@ void mbedtls_x509write_crt_set_version(mbedtls_x509write_cert *ctx, int version) * input buffer * * \return 0 if successful, or - * #MBEDTLS_ERR_X509_BAD_INPUT_DATA if the provided input buffer - * is too big (longer than MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) + * #MBEDTLS_ERR_X509_BAD_INPUT_DATA if \p serial_len is 0 or + * too big (longer than MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) */ int mbedtls_x509write_crt_set_serial_raw(mbedtls_x509write_cert *ctx, const unsigned char *serial, size_t serial_len); diff --git a/library/x509write_crt.c b/library/x509write_crt.c index 8c77f10c34..74f27af1cc 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -95,7 +95,7 @@ int mbedtls_x509write_crt_set_issuer_name(mbedtls_x509write_cert *ctx, int mbedtls_x509write_crt_set_serial_raw(mbedtls_x509write_cert *ctx, const unsigned char *serial, size_t serial_len) { - if (serial_len > MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) { + if (serial_len == 0 || serial_len > MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) { return MBEDTLS_ERR_X509_BAD_INPUT_DATA; } @@ -512,7 +512,18 @@ int mbedtls_x509write_crt_der(mbedtls_x509write_cert *ctx, * - if MSb of "serial" is 1, then prepend an extra 0x00 byte * - 1 byte for the length * - 1 byte for the TAG + * + * ctx->serial_len == 0 (the zeroed state left by + * mbedtls_x509write_crt_init() if mbedtls_x509write_crt_set_serial_raw() + * was never called) must be rejected here: an INTEGER with zero content + * octets is not valid DER (X.690 8.3.1 requires at least one octet), so + * letting it through would silently write a syntactically invalid + * `02 00` CertificateSerialNumber field instead of failing loudly. */ + if (ctx->serial_len == 0) { + return MBEDTLS_ERR_X509_BAD_INPUT_DATA; + } + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_raw_buffer(&c, buf, ctx->serial, ctx->serial_len)); if (*c & 0x80) { diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 760ff5fe03..8952817cd6 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -565,6 +565,7 @@ void x509_set_serial_check() { mbedtls_x509write_cert ctx; uint8_t invalid_serial[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN + 1]; + uint8_t zero_len_serial[1] = { 0x2a }; USE_PSA_INIT(); memset(invalid_serial, 0x01, sizeof(invalid_serial)); @@ -573,6 +574,12 @@ void x509_set_serial_check() sizeof(invalid_serial)), MBEDTLS_ERR_X509_BAD_INPUT_DATA); + /* A zero-length serial must also be rejected: an ASN.1 INTEGER with + * zero content octets is not valid DER (X.690 8.3.1), so + * mbedtls_x509write_crt_der() must never be allowed to encode one. */ + TEST_EQUAL(mbedtls_x509write_crt_set_serial_raw(&ctx, zero_len_serial, 0), + MBEDTLS_ERR_X509_BAD_INPUT_DATA); + exit: ; USE_PSA_DONE();