Fixed encode policy oid

[stenslae]

Description

Fixed a bug in EncodePolicyOID where combined first identifiers (the first two OID arcs) were improperly cast to a single byte when exceeding 127, rather than using the required ITU-T X.690 base-128 continuation encoding. Also added improved bounds checking.

Testing

Added new test coverage to exercise these changes.

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m4-crypto-only

• FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +43 B (+0.0%, 175,385 B / 262,144 B, total: 67% used)

gcc-arm-cortex-m4-pkcs7

• FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +43 B, .text -160 B (-0.1%, 214,082 B / 262,144 B, total: 82% used)

gcc-arm-cortex-m4-pq

• FLASH: .rodata +52 B, .text -208 B (-0.1%, 296,768 B / 1,048,576 B, total: 28% used)

gcc-arm-cortex-m4-tls13

• FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +43 B (+0.0%, 237,712 B / 262,144 B, total: 91% used)

gcc-arm-cortex-m7

• FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +43 B (+0.0%, 201,438 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m7-pq

• FLASH: .rodata +52 B, .text -144 B (-0.0%, 297,728 B / 1,048,576 B, total: 28% used)

gcc-arm-cortex-m7-tls13

• FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +43 B (+0.0%, 237,776 B / 262,144 B, total: 91% used)

linuxkm-pie

• Data: __patchable_function_entries +24 B (+0.1%, 26,464 B)

linuxkm-standard

• Data: __patchable_function_entries +16 B (+0.0%, 49,224 B)
  No memory changes detected for:
• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[stenslae]

jenkins retest this please

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[dgarske]

The new DecodeOidArc() applies its non-minimal-encoding check (cnt == 0 && in[*idx] == 0x80) and its word32-overflow check (v > (0xFFFFFFFFU >> 7)) unconditionally, i.e. to every arc, not just the first identifier. The strict flag only changes what happens when the input ends mid-continuation. The old loop tolerated both of these for arcs after the first identifier: a leading 0x80 simply contributed 0, and an overflowing shift silently wrapped. Because DecodeCertPolicy() (asn.c:21443) treats any <= 0 return as fatal and returns ASN_PARSE_E, a certificate carrying such a policy OID now fails to parse entirely where it previously parsed. I confirmed this against the built library: {0x2a, 0x80, 0x03} used to decode to "1.2.3" and now returns -144 (ASN_OBJECT_ID_E); {0x2a, 0x90, 0x80, 0x80, 0x80, 0x00} (arc == 2^32) used to render as ".0" and now returns -144. Rejecting is the more correct DER posture and is probably intended, but the PR body only describes an encoder fix plus "improved bounds checking", and the new doxygen note on wc_DecodePolicyOID only mentions the first-identifier strictness and the 2.40-2.47 rendering change.

Recommendation: Confirm the parse-side strictness is intended, then (a) extend the wc_DecodePolicyOID note to cover later arcs

[stenslae]

Confirmed the parse-side strictness is intended and added fixes on the decode route.

[dgarske]

DecodeOidArc()'s strict parameter changes behavior in exactly one place: end-of-input mid-continuation returns ASN_OBJECT_ID_E when strict and ASN_OID_ARC_TRUNCATED (silently dropping the partial arc) when not. Test 16 covers only the strict side ({0x81, 0x81} as a first identifier). Nothing in the new suite exercises a truncated trailing arc, so the if (ret == ASN_OID_ARC_TRUNCATED) break; path in wc_DecodePolicyOID and the return strict ? ... : ASN_OID_ARC_TRUNCATED line are both untested. I verified the intended behavior against the built library: {0x2a, 0x81} returns 3 with "1.2". Similarly, the newly-strict later-arc paths described in the previous finding (non-minimal {0x2a, 0x80, 0x03} and overflow {0x2a, 0x90, 0x80, 0x80, 0x80, 0x00}, both -144) have no assertions, so a future relaxation of that behavior would pass CI silently. The heap != NULL path through wc_EncodePolicyOID (which is what every in-tree caller except the OpenSSL-compat layer uses) is also never exercised.

Recommendation: Add the truncated-trailing-arc, non-minimal-later-arc, overflow-later-arc, and max-word32-arc cases above, plus at least one wc_EncodePolicyOID call with a non-NULL heap so the XMALLOC/XFREE pairing on the heap-hint path is exercised.

[stenslae]

Added suggested test coverage

[dgarske]

The new empty-arc rejection calls XSTRSTR(in, ".."). Grepping the tree, this is the only XSTRSTR use in wolfcrypt/src/*.c, and wolfssl/wolfcrypt/types.h:1021-1023 carries an explicit note directly above the definition: "strstr, strncmp, strcmp, and strncat only used by wolfSSL proper, not required for wolfCrypt only". Ports that build with STRING_USER and supply only the wolfCrypt-required X-macros will now fail to compile wolfcrypt/src/asn.c under --enable-certgen --enable-certext, a wolfCrypt-only configuration. The check is also redundant work: the function already walks the whole string in ParseOidArc per token, and the three pre-scans (XSTRLEN, in[0], in[len-1], XSTRSTR) can collapse into one pass.

Recommendation: Replace XSTRSTR with an inline scan so wolfcrypt/src/asn.c does not gain a dependency on a string routine that types.h documents as wolfSSL-proper-only.

[stenslae]

Used inline scan instead

[dgarske]

We actually do not need the old macros because they were WOLFSSL_LOCAL. If anywhere else in our code base is using them we should rename those. If customers are using this in a static build (for example) they will need to update their code.

[stenslae]

The old macros have been removed and any remaining internal usages were renamed to use the wc_ prefix.

[dgarske]

The PR renamed EncodePolicyOID -> wc_EncodePolicyOID at asn_orig.c:5550 but left DecodePolicyOID at asn_orig.c:4227 on the old spelling, relying on the new header macro. Same file, same PR, two conventions. asn_orig.c is #included into asn.c (asn.c:39546) so it compiles either way, but the inconsistency is the reason the compat macros cannot be deleted.

Recommendation: Rename this call site to wc_DecodePolicyOID to match the other in-tree call sites updated by this PR.

[stenslae]

Changed to wc_

[dgarske]

Recommendation: Either rename out of the ASN_* namespace and #undef at the end of the guarded block, or replace the sentinel with an int* truncated out-parameter so the function keeps asn.c's usual 0/negative return contract.

[stenslae]

Replaced the sentinel with an int* truncated out-parameter to maintain the standard 0/negative return contract.

[dgarske]

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]

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.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 8
8 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11019

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 7
7 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

🟠 [Medium] Successful wc_EncodePolicyOID() clears a previously set encoding error in EncodeExtensions() · Incorrect error handling

ret can already hold KEYUSAGE_E (line 28982) or CERTPOLICIES_E (line 29012) inside the same if (ret == 0) block; the new unconditional assignment resets it to 0 on success. The sizing pass (EncodeExtensions(cert, NULL, 0, 0) at line 30116) then returns a bogus extension size instead of the error, since the recovery checks at lines 29219/29230 only run when output != NULL.

Fix: Gate the custom-extension loop on ret == 0, or assign the encode result to a local and only propagate it into ret on failure.

[wolfSSL-Fenrir-bot]

🔵 [Low] Malformed policy OID arc is now silently dropped instead of rejecting the certificate · X.509 and ASN.1 parsing vulnerabilities

ASN_OID_ARC_TOO_BIG_E sets skipPolicy instead of failing, so an attacker-supplied certificate with an over-long or word32-overflowing policy arc now parses successfully with that entry omitted from extCertPolicies. Previously the same input returned ASN_PARSE_E. The same relaxation is duplicated at asn_orig.c:4241.

Related known finding #7309 (similar but distinct): Both relax ASN.1 validation and permit malformed/non-canonical certificate input, but this is DecodeCertPolicy's oversized-OID-arc handling while 7309 is INTEGER encoding validation gated by a build macro. The faulting operations and root causes differ, and each requires a separate patch.

Fix: Restrict the skip to well-formed magnitude overflow only, or record a flag on DecodedCert so callers can tell the policy list is incomplete.

[wolfSSL-Fenrir-bot]

🔵 [Low] New small-buffer tests never assert the fail-closed NUL-termination the PR adds · Weak or missing assertions

The three new OBJ_obj2txt() small-buffer cases check only the return value, not that the output buffer is left as an empty NUL-terminated string. Removing the new buf[0] = '\0' writes in wolfssl_obj2txt_numeric() (src/ssl_asn1.c:5868, 5894) leaves every one of these assertions passing.

Fix: Add ExpectIntEQ(smallBuf[0], '\0') after each failing OBJ_obj2txt() call.

[wolfSSL-Fenrir-bot]

🔵 [Low] DER patch search scans the whole certificate, including the random serial and signature · Buffer safety in test payload builders

The 3-byte pattern {0x81,0x34,0x03} is searched across the entire DER produced by wc_MakeSelfCert(), whose serial number and signature differ every run. A chance match ahead of the policy OID corrupts an unrelated byte, leaving the policy intact and making the wc_ParseCert() == ASN_PARSE_E assertion fail nondeterministically.

Fix: Search for the full OID TLV {0x06, 0x03, 0x81, 0x34, 0x03} instead of the bare content bytes.

[wolfSSL-Fenrir-bot]

⚪ [Info] Changed obj2txt failure guard in wolfssl_x509_add_custom_ext has no test coverage · Missing edge-case coverage on a function the PR also changed

The guard was widened from < 0 to <= 0 so that a wolfSSL_OBJ_obj2txt() return of exactly 0 no longer stores the uninitialized oid allocation into x509->custom_exts. No test in this PR drives wolfSSL_X509_add_ext() with an extension object whose OID fails to convert, so the newly covered branch is unexercised.

Fix: Add a WOLFSSL_CUSTOM_OID test that calls wolfSSL_X509_add_ext() with an unrecognized-NID extension carrying a malformed OBJECT IDENTIFIER.

[wolfSSL-Fenrir-bot]

🔵 [Low] New OBJ_obj2txt small-buffer tests never assert the fail-closed buffer contract · Weak or missing assertions

The three new small-buffer cases check only the WOLFSSL_FAILURE return code. smallBuf is left uninitialized and never inspected, so the buf[0] = '\0' fail-closed guarantee this PR added to wolfssl_obj2txt_numeric (src/ssl_asn1.c:5868, 5894) is unverified — a regression leaving stale stack contents in the caller's buffer would still pass.

Fix: Pre-fill each smallBuf with a non-NUL sentinel and assert smallBuf[0] == '\0' after each failing call.

[wolfSSL-Fenrir-bot]

⚪ [Info] Tightened wolfssl_x509_add_custom_ext return check has no test coverage · Missing edge-case coverage on a function the PR also changed

The < 0 to <= 0 change closes the path where a WOLFSSL_FAILURE (0) return let the uninitialized oid allocation be stored in x509->custom_exts[] and later read as a C string. No test drives wolfSSL_X509_add_ext with a custom-OID extension whose OBJ_obj2txt fails, so this branch stays uncovered.

Fix: Add a WOLFSSL_CUSTOM_OID test that adds an extension with an undecodable OBJECT IDENTIFIER and asserts wolfSSL_X509_add_ext returns failure.