Fixed encode policy oid by stenslae · Pull Request #11019 · wolfSSL/wolfssl

stenslae · 2026-07-30T16:04:22Z

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 · 2026-07-30T16:33:06Z

MemBrowse Memory Report

gcc-arm-cortex-m4-openssl-compat

FLASH: .rodata +8 B, .text +192 B (+0.0%, 773,044 B / 1,048,576 B, total: 74% used)

gcc-arm-cortex-m4-pkcs7

FLASH: .text +1,536 B (+0.7%, 215,450 B / 262,144 B, total: 82% used)

gcc-arm-cortex-m4-pq

FLASH: .rodata +8 B, .text -208 B (-0.1%, 280,888 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

FLASH: .rodata +8 B, .text -192 B (-0.1%, 327,008 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m7-pq

FLASH: .rodata +8 B, .text -208 B (-0.1%, 281,464 B / 1,048,576 B, total: 27% 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,240 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-crypto-only
gcc-arm-cortex-m4-dtls13
gcc-arm-cortex-m4-min-ecc
gcc-arm-cortex-m4-sp-math
gcc-arm-cortex-m4-tls12
gcc-arm-cortex-m4-tls13
gcc-arm-cortex-m7
gcc-arm-cortex-m7-tls13
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 · 2026-07-31T15:36:25Z

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 · 2026-08-06T04:14:29Z

+
+    while (*idx < inSz) {
+        if (in[*idx] & 0x80) {
+            if (cnt == 0 && in[*idx] == 0x80)


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

dgarske · 2026-08-06T04:16:29Z

+             * whatever partial value had accumulated so far. */
+            {
+                const byte truncated[] = { 0x81, 0x81 };
+                ExpectIntLT(DecodePolicyOID(decoded, sizeof(decoded),


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.

dgarske · 2026-08-06T04:17:40Z

    len = (word32)XSTRLEN(in);
+    if (len == 0 || in[0] == '.' || in[len - 1] == '.')
+        return ASN_OBJECT_ID_E;
+    if (XSTRSTR(in, "..") != NULL)


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.

dgarske · 2026-08-06T04:18:47Z

+WOLFSSL_TEST_VIS int wc_EncodePolicyOID(byte *out, word32 *outSz,
+                                         const char *in, void* heap);
+/* Deprecated public API names kept for backwards build compatibility */
+#define DecodePolicyOID(out, outSz, in, inSz) \


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.

dgarske · 2026-08-06T04:19:22Z

@@ -5547,7 +5547,7 @@ static int SetCertificatePolicies(byte *output,
        oidSz = sizeof(oid);
        XMEMSET(oid, 0, oidSz);

-        ret = EncodePolicyOID(oid, &oidSz, input[i], heap);
+        ret = wc_EncodePolicyOID(oid, &oidSz, input[i], heap);


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.

dgarske · 2026-08-06T04:20:01Z

@@ -21229,58 +21229,114 @@ static int DecodeNameConstraints(const byte* input, word32 sz,
 #if defined(WOLFSSL_CERT_EXT) || \
    defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)

+/* returned by DecodeOidArc() (in place of 0/ASN_OBJECT_ID_E) when the
+ * input ends mid-continuation with no terminating byte and strict==0 */
+#define ASN_OID_ARC_TRUNCATED 1


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.

dgarske · 2026-08-06T04:21:03Z

        }

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


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 requested a review from wolfSSL-Fenrir-bot July 30, 2026 16:04

stenslae self-assigned this Jul 30, 2026

wolfSSL-Fenrir-bot reviewed Jul 30, 2026

View reviewed changes

stenslae force-pushed the fix-encodepolicyoid-multibyte-arc branch from 49ac912 to 6755e0b Compare July 30, 2026 16:32

stenslae requested a review from wolfSSL-Fenrir-bot July 30, 2026 16:33

stenslae requested review from wolfSSL-Fenrir-bot and removed request for wolfSSL-Fenrir-bot July 30, 2026 16:47

wolfSSL-Fenrir-bot reviewed Jul 30, 2026

View reviewed changes

stenslae removed their assignment Jul 30, 2026

stenslae force-pushed the fix-encodepolicyoid-multibyte-arc branch from 6755e0b to 47a52ac Compare July 30, 2026 19:21

stenslae requested a review from wolfSSL-Fenrir-bot July 30, 2026 19:21

wolfSSL-Fenrir-bot reviewed Jul 30, 2026

View reviewed changes

stenslae assigned wolfSSL-Bot Jul 31, 2026

dgarske requested changes Aug 5, 2026

View reviewed changes

Comment thread wolfcrypt/src/asn.c

Comment thread wolfcrypt/src/asn.c Outdated

Comment thread wolfcrypt/src/asn.c

Comment thread wolfcrypt/src/asn.c Outdated

Comment thread wolfcrypt/src/asn.c Outdated

dgarske assigned stenslae Aug 5, 2026

stenslae force-pushed the fix-encodepolicyoid-multibyte-arc branch from 47a52ac to 9aee0e8 Compare August 5, 2026 21:50

stenslae requested a review from wolfSSL-Fenrir-bot August 5, 2026 21:51

wolfSSL-Fenrir-bot reviewed Aug 5, 2026

View reviewed changes

Comment thread tests/api/test_asn.c

Comment thread wolfcrypt/src/asn.c Outdated

stenslae commented Aug 5, 2026

View reviewed changes

Comment thread wolfcrypt/src/asn.c Outdated

stenslae commented Aug 5, 2026

View reviewed changes

Comment thread tests/api/test_asn.c

fixed encode policy oid

c07964a

stenslae force-pushed the fix-encodepolicyoid-multibyte-arc branch from 9aee0e8 to c07964a Compare August 5, 2026 22:28

stenslae requested a review from wolfSSL-Fenrir-bot August 5, 2026 22:28

wolfSSL-Fenrir-bot reviewed Aug 5, 2026

View reviewed changes

dgarske unassigned stenslae Aug 6, 2026

dgarske self-requested a review August 6, 2026 00:37

dgarske requested changes Aug 6, 2026

View reviewed changes

dgarske assigned stenslae Aug 6, 2026

Uh oh!

Conversation

stenslae commented Jul 30, 2026

Description

Testing

Checklist

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11019

Uh oh!

github-actions Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11019

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11019

Uh oh!

stenslae commented Jul 31, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jul 30, 2026 •

edited

Loading