Skip to content

20260602-FPKI-DecodeGeneralName-URI#10578

Merged
Frauschi merged 1 commit into
wolfSSL:masterfrom
douzzer:20260602-FPKI-DecodeGeneralName-URI
Jun 3, 2026
Merged

20260602-FPKI-DecodeGeneralName-URI#10578
Frauschi merged 1 commit into
wolfSSL:masterfrom
douzzer:20260602-FPKI-DecodeGeneralName-URI

Conversation

@douzzer
Copy link
Copy Markdown
Contributor

@douzzer douzzer commented Jun 2, 2026

wolfcrypt/src/asn.c: don't disable URI validation in DecodeGeneralName() and DecodeAcertGeneralName() when defined(WOLFSSL_FPKI).

tests/api/test_certman.c: fix uriSan in test_wolfSSL_X509_check_host_URI_SAN_not_DNS_match() (make it a URI).

tests/api.c: align gating in test_wolfSSL_URI() with new dynamics (URIs validated regardless of defined(WOLFSSL_FPKI)).

detected and tested with

wolfssl-multi-test.sh ...
old-valgrind-config-sanitizer
check-source-text
clang-tidy-all-sp-all

Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10578

Scan targets checked: wolfcrypt-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 2

Medium (2)

URI slash error path reads past payload

File: wolfcrypt/src/asn.c:18663
Function: DecodeGeneralName
Category: Logic errors

DecodeGeneralName() now enables this validator for WOLFSSL_FPKI, but the / path sets i = len inside the for; the loop increment makes i > len, so the later URI check reads past the GeneralName payload.

Recommendation: Return ASN_ALT_NAME_E immediately on /, or break using a separate malformed flag instead of mutating i.

Referenced code: wolfcrypt/src/asn.c:18663-18667 (5 lines)


ACERT URI slash error path reads past payload

File: wolfcrypt/src/asn.c:37745
Function: DecodeAcertGeneralName
Category: Logic errors

DecodeAcertGeneralName() now enables this validator for WOLFSSL_FPKI, but the / path sets i = len inside the for; the loop increment makes i > len, so the later URI check reads past the GeneralName payload.

Recommendation: Return ASN_ALT_NAME_E immediately on /, or break using a separate malformed flag instead of mutating i.

Referenced code: wolfcrypt/src/asn.c:37745-37749 (5 lines)


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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

MemBrowse Memory Report

No memory changes detected for:

…e(),

* don't disable URI validation when defined(WOLFSSL_FPKI).
* return immediately with ASN_ALT_NAME_E when URI contains an unexpected '/', as in asn_orig.c DecodeAltNames(), fixing OOB read defect.

wolfcrypt/src/asn_orig.c: fix URI validation gating (ignore WOLFSSL_FPKI) in DecodeAltNames().

tests/api/test_certman.c: fix uriSan in test_wolfSSL_X509_check_host_URI_SAN_not_DNS_match() (make it a URI).

tests/api.c: align gating in test_wolfSSL_URI() with new dynamics (URIs validated regardless of defined(WOLFSSL_FPKI)).
@douzzer douzzer force-pushed the 20260602-FPKI-DecodeGeneralName-URI branch from 0457204 to 768cdc3 Compare June 3, 2026 03:17
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10578

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

No new issues found in the changed files. ✅

@douzzer
Copy link
Copy Markdown
Contributor Author

douzzer commented Jun 3, 2026

retest this please
(Jenkins glitch)

@Frauschi
Copy link
Copy Markdown
Contributor

Frauschi commented Jun 3, 2026

The changes to test_certman.c conflict with #10573 (I thought that the missing http:// has been intentional). I'll close #10573 if your approach is the right one.

@Frauschi Frauschi merged commit e80b4b5 into wolfSSL:master Jun 3, 2026
461 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For This Release Release version 5.9.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants