From cdaee54773cbba892afce2b64bbcd7acd13784ac Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 14 Feb 2023 14:34:15 +0000 Subject: [PATCH] Fix incorrect printing of OIDs The first 2 components of an OID are combined together into the same subidentifier via the formula: subidentifier = (component1 * 40) + component2 The current code extracts component1 and component2 using division and modulo as one would expect. However, there is a subtlety in the specification[1]: >This packing of the first two object identifier components recognizes >that only three values are allocated from the root node, and at most >39 subsequent values from nodes reached by X = 0 and X = 1. If the root node (component1) is 2, the subsequent node (component2) may be greater than 38. For example, the following are real OIDs: * 2.40.0.25, UPU standard S25 * 2.49.0.0.826.0, Met Office * 2.999, Allocated example OID This has 2 implications that the current parsing code does not take account of: 1. The second component may be > 39, so (subidentifier % 40) is not correct in all circumstances. 2. The first subidentifier (containing the first 2 components) may be more than one byte long. Currently we assume it is just 1 byte. Improve parsing code to deal with these cases correctly. [1] Rec. ITU-T X.690 (02/2021), 8.19.4 Signed-off-by: David Horstmann --- library/oid.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/library/oid.c b/library/oid.c index fcff15273..d8ba773a5 100644 --- a/library/oid.c +++ b/library/oid.c @@ -796,14 +796,39 @@ 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; + 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; + } + + value += oid->p[i] & 0x7F; + value <<= 7; + i++; } + if (i >= oid->len) { + return MBEDTLS_ERR_OID_BUF_TOO_SMALL; + } + /* 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 = 1; i < oid->len; i++) { + for (; i < oid->len; i++) { /* Prevent overflow in value. */ if (((value << 7) >> 7) != value) { return MBEDTLS_ERR_OID_BUF_TOO_SMALL;