Fix off-by-one in pem_find_pattern rejecting PEM without a trailing newline - #11259
Fix off-by-one in pem_find_pattern rejecting PEM without a trailing newline#11259gasbytes wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch? |
380cdda to
48a29f1
Compare
|
retest this please |
|
Jenkins retest this please (logs lost) |
48a29f1 to
7e054bc
Compare
|
Jenkins retest this please (timeout) |
1 similar comment
|
Jenkins retest this please (timeout) |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11259
Scan targets checked: wolfssl-bugs, wolfssl-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
| } | ||
| /* Find postfix part. */ | ||
| for (; idx < pemLen - postfixLen; idx++) { | ||
| for (; idx <= pemLen - postfixLen; idx++) { |
There was a problem hiding this comment.
Widened postfix scan makes PEM ending in a blank line fail to parse · Logic errors
The header search at src/pk.c:6905 uses postfix "\n\n" and is not bounded by the footer. With idx <= pemLen - postfixLen, a trailing blank line at the very end of the input now matches, so hdrLen swallows the body and footer, start advances to pemLen, the footer search finds nothing, and pem_read_data returns ASN_NO_PEM_HEADER. Unencrypted PEM files ending in "\n\n" that parsed before this PR now fail in PEM_read/PEM_read_bio.
Related known finding #11685 (similar but distinct): Both affect boundary handling in pem_find_pattern, but #11685 concerns excluding a valid footer match at the buffer end due to a strict bound. This candidate instead lets the header postfix scan match a blank line after the footer because its search is not footer-bounded. The operations, root causes, and required fixes differ.
Fix: Bound the encryption-header search so it cannot match past the footer — locate PEM_END first and pass that offset as the search limit.
There was a problem hiding this comment.
this feels like a false positive but not sure
There was a problem hiding this comment.
seems legit imo, should be addressed now.
|
skoll locally ran clean, no comments. |
…he footers's final dash
7e054bc to
a71e104
Compare
|
Jenkins retest this please |
4 participants
Description
Changed both scans loops in pem_find_pattern to <= so PEM ending at the footer's final dash parses.
Testing
Updated the general test test_wolfssl_pem_read to test this edge case too.
Checklist