diff --git a/ChangeLog.d/fix-oid-to-string-bugs.txt b/ChangeLog.d/fix-oid-to-string-bugs.txt new file mode 100644 index 000000000..799f44474 --- /dev/null +++ b/ChangeLog.d/fix-oid-to-string-bugs.txt @@ -0,0 +1,6 @@ +Bugfix + * Fix bug in conversion from OID to string in + mbedtls_oid_get_numeric_string(). OIDs such as 2.40.0.25 are now printed + correctly. + * Reject OIDs with overlong-encoded subidentifiers when converting + OID-to-string. diff --git a/library/oid.c b/library/oid.c index e7c12248a..86214b23a 100644 --- a/library/oid.c +++ b/library/oid.c @@ -834,21 +834,55 @@ int mbedtls_oid_get_numeric_string(char *buf, size_t size, p = buf; n = size; - /* First byte contains first two dots */ - if (oid->len > 0) { - ret = mbedtls_snprintf(p, n, "%d.%d", oid->p[0] / 40, oid->p[0] % 40); - OID_SAFE_SNPRINTF; + /* First subidentifier contains first two OID components */ + i = 0; + value = 0; + if ((oid->p[0]) == 0x80) { + /* Overlong encoding is not allowed */ + return MBEDTLS_ERR_ASN1_INVALID_DATA; } - value = 0; - for (i = 1; i < oid->len; i++) { + while (i < oid->len && ((oid->p[i] & 0x80) != 0)) { /* Prevent overflow in value. */ - if (((value << 7) >> 7) != value) { - return MBEDTLS_ERR_OID_BUF_TOO_SMALL; + if (value > (UINT_MAX >> 7)) { + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + + value |= oid->p[i] & 0x7F; + value <<= 7; + i++; + } + if (i >= oid->len) { + return MBEDTLS_ERR_ASN1_OUT_OF_DATA; + } + /* Last byte of first subidentifier */ + value |= oid->p[i] & 0x7F; + i++; + + unsigned int component1 = value / 40; + if (component1 > 2) { + /* The first component can only be 0, 1 or 2. + * If oid->p[0] / 40 is greater than 2, the leftover belongs to + * the second component. */ + component1 = 2; + } + unsigned int component2 = value - (40 * component1); + ret = mbedtls_snprintf(p, n, "%u.%u", component1, component2); + OID_SAFE_SNPRINTF; + + value = 0; + for (; i < oid->len; i++) { + /* Prevent overflow in value. */ + if (value > (UINT_MAX >> 7)) { + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + if ((value == 0) && ((oid->p[i]) == 0x80)) { + /* Overlong encoding is not allowed */ + return MBEDTLS_ERR_ASN1_INVALID_DATA; } value <<= 7; - value += oid->p[i] & 0x7F; + value |= oid->p[i] & 0x7F; if (!(oid->p[i] & 0x80)) { /* Last byte */ diff --git a/tests/suites/test_suite_oid.data b/tests/suites/test_suite_oid.data index 1738841d5..b9fa6543d 100644 --- a/tests/suites/test_suite_oid.data +++ b/tests/suites/test_suite_oid.data @@ -89,3 +89,33 @@ oid_get_md_alg_id:"2b24030201":MBEDTLS_MD_RIPEMD160 OID hash id - invalid oid oid_get_md_alg_id:"2B864886f70d0204":-1 +OID get numeric string - hardware module name +oid_get_numeric_string:"2B06010505070804":0:"1.3.6.1.5.5.7.8.4" + +OID get numeric string - multi-byte subidentifier +oid_get_numeric_string:"29903C":0:"1.1.2108" + +OID get numeric string - second component greater than 39 +oid_get_numeric_string:"81010000863A00":0:"2.49.0.0.826.0" + +OID get numeric string - multi-byte first subidentifier +oid_get_numeric_string:"8837":0:"2.999" + +OID get numeric string - empty oid buffer +oid_get_numeric_string:"":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" + +OID get numeric string - no final / all bytes have top bit set +oid_get_numeric_string:"818181":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" + +# Encodes the number 0x0400000000 as a subidentifier which overflows 32-bits +OID get numeric string - 32-bit overflow +oid_get_numeric_string:"C080808000":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID get numeric string - 32-bit overflow, second subidentifier +oid_get_numeric_string:"2BC080808000":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID get numeric string - overlong encoding +oid_get_numeric_string:"8001":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID get numeric string - overlong encoding, second subidentifier +oid_get_numeric_string:"2B8001":MBEDTLS_ERR_ASN1_INVALID_DATA:"" diff --git a/tests/suites/test_suite_oid.function b/tests/suites/test_suite_oid.function index 687b2168a..3004b65fe 100644 --- a/tests/suites/test_suite_oid.function +++ b/tests/suites/test_suite_oid.function @@ -96,3 +96,24 @@ void oid_get_md_alg_id(data_t *oid, int exp_md_id) } } /* END_CASE */ + +/* BEGIN_CASE */ +void oid_get_numeric_string(data_t *oid, int error_ret, char *result_str) +{ + char buf[256]; + mbedtls_asn1_buf input_oid = { 0, 0, NULL }; + int ret; + + input_oid.tag = MBEDTLS_ASN1_OID; + input_oid.p = oid->x; + input_oid.len = oid->len; + + ret = mbedtls_oid_get_numeric_string(buf, sizeof(buf), &input_oid); + + if (error_ret == 0) { + TEST_ASSERT(strcmp(buf, result_str) == 0); + } else { + TEST_EQUAL(ret, error_ret); + } +} +/* END_CASE */ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 961b25ac1..4545a5390 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -2558,7 +2558,7 @@ X509 OID numstring #4 (larger number) x509_oid_numstr:"2a864886f70d":"1.2.840.113549":15:14 X509 OID numstring #5 (arithmetic overflow) -x509_oid_numstr:"2a8648f9f8f7f6f5f4f3f2f1f001":"":100:MBEDTLS_ERR_OID_BUF_TOO_SMALL +x509_oid_numstr:"2a8648f9f8f7f6f5f4f3f2f1f001":"":100:MBEDTLS_ERR_ASN1_INVALID_DATA X509 CRT keyUsage #1 (no extension, expected KU) depends_on:MBEDTLS_RSA_C:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA