Skip to content

Fixed encode policy oid #11019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stenslae wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed encode policy oid #11019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions src/ssl_asn1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5845,14 +5845,16 @@ int wc_OBJ_sn2nid(const char *sn)
* buffer.
*
* String is of the form "1.2.840.113549.1.9.1" and is always NUL
* terminated. Truncated when the buffer is too small.
* terminated. Unlike OpenSSL's OBJ_obj2txt(), fails outright on a too
* small buffer rather than truncating.
*
* @param [out] buf Buffer to hold string.
* @param [in] bufLen Length of buffer in bytes.
* @param [in] a ASN.1 OBJECT_ID object.
* @return Length of string that would be written, excluding the NUL
* terminator, on success.
* @return 0 when decoding the object fails.
* @return ASN_PARSE_E when the object's length cannot be parsed.
*/
static int wolfssl_obj2txt_numeric(char *buf, int bufLen,
const WOLFSSL_ASN1_OBJECT *a)
Expand All @@ -5862,6 +5864,9 @@ int wc_OBJ_sn2nid(const char *sn)
word32 idx = 0;
byte tag;

/* Fail closed: buf is always NUL terminated, even on error. */
buf[0] = '\0';

if (GetASNTag(a->obj, &idx, &tag, a->objSz) != 0) {
return WOLFSSL_FAILURE;
}
Expand All @@ -5876,17 +5881,14 @@ int wc_OBJ_sn2nid(const char *sn)
return ASN_PARSE_E;
}

/* save an extra byte for null term. */
if (bufLen < MAX_OID_STRING_SZ) {
bufSz = bufLen - 1;
}
else {
bufSz = MAX_OID_STRING_SZ - 1;
}
/* wc_DecodePolicyOID() accounts for the NUL terminator itself; pass
* the full buffer length as-is, don't clamp to MAX_OID_STRING_SZ. */
bufSz = bufLen;

if ((bufSz = DecodePolicyOID(buf, (word32)bufSz, a->obj + idx,
if ((bufSz = wc_DecodePolicyOID(buf, (word32)bufSz, a->obj + idx,

@dgarske dgarske Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc comment states "String is of the form ... and is always NUL terminated. Truncated when the buffer is too small." The PR tightens wc_DecodePolicyOID's length checks from w > outSz - outIdx to w >= outSz - outIdx and adds a bound check on the first XSNPRINTF that had none, so the exact-fit case that previously produced a silently truncated (and therefore wrong) OID string now returns BUFFER_E, which this function maps to WOLFSSL_FAILURE. The stricter behavior is the right call - a silently shortened OID string is worse than an error, and the missing first-XSNPRINTF check was an out-of-bounds out[outIdx] = 0 write when the first identifier alone overflowed the caller's buffer - but the comment now describes behavior the code no longer has, and it diverges from OpenSSL's OBJ_obj2txt, which truncates and returns the length that would have been needed.

Code:

         * String is of the form "1.2.840.113549.1.9.1" and is always NUL
         * terminated. Truncated when the buffer is too small.
         ...
         * @return  0 when decoding the object fails.

Recommendation: Update the comment to say the call fails rather than truncates on a short buffer, and note the OpenSSL-compat divergence if it matters for callers of wolfSSL_OBJ_obj2txt(buf, small, obj, 1).

@stenslae stenslae Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the doc comment to explicitly state that the function will fail and return BUFFER_E when the buffer is too small. I also added a note regarding the divergence from OpenSSL's OBJ_obj2txt behavior.

Comment thread
stenslae marked this conversation as resolved.
Comment thread
stenslae marked this conversation as resolved.
(word32)length)) <= 0) {
WOLFSSL_MSG("Error decoding OID");
buf[0] = '\0';
return WOLFSSL_FAILURE;
}

Expand Down Expand Up @@ -6259,7 +6261,7 @@ int wc_OBJ_sn2nid(const char *sn)
}

#ifdef WOLFSSL_CERT_EXT
ret = EncodePolicyOID(out, &outSz, s, NULL);
ret = wc_EncodePolicyOID(out, &outSz, s, NULL);
if (ret == 0) {
/* sum OID */
sum = wc_oid_sum(out, outSz);
Expand Down Expand Up @@ -6324,7 +6326,7 @@ int wc_OBJ_sn2nid(const char *sn)
return NULL;

/* If s is numerical value, try to sum oid */
ret = EncodePolicyOID(out, &outSz, s, NULL);
ret = wc_EncodePolicyOID(out, &outSz, s, NULL);
if (ret == 0 && outSz > 0) {
/* If numerical encode succeeded then just
* create object from that because sums are
Expand Down
2 changes: 1 addition & 1 deletion src/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ static int wolfssl_x509_add_custom_ext(WOLFSSL_X509 *x509,

if (err == 0) {
XMEMCPY(val, ext->value.data, ext->value.length);
if (wolfSSL_OBJ_obj2txt(oid, MAX_OID_STRING_SZ, ext->obj, 1) < 0) {
if (wolfSSL_OBJ_obj2txt(oid, MAX_OID_STRING_SZ, ext->obj, 1) <= 0) {
Comment thread
stenslae marked this conversation as resolved.
Comment thread
stenslae marked this conversation as resolved.
err = 1;
}
}
Expand Down
Loading
Loading