Merge pull request #7105 from davidhorstmann-arm/fix-oid-printing-bug
Fix bugs in OID to string conversion
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 0000000..799f444
--- /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 e7c1224..86214b2 100644
--- a/library/oid.c
+++ b/library/oid.c
@@ -834,21 +834,55 @@
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 1738841..b9fa654 100644
--- a/tests/suites/test_suite_oid.data
+++ b/tests/suites/test_suite_oid.data
@@ -89,3 +89,33 @@
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 687b216..3004b65 100644
--- a/tests/suites/test_suite_oid.function
+++ b/tests/suites/test_suite_oid.function
@@ -96,3 +96,24 @@
}
}
/* 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 961b25a..4545a53 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -2558,7 +2558,7 @@
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