Skip to content

Add bounds check in PKCS7 streaming indefinite-length end-of-content parsing#10039

Merged
douzzer merged 2 commits intowolfSSL:masterfrom
anhu:pkcs7_oob
Apr 2, 2026
Merged

Add bounds check in PKCS7 streaming indefinite-length end-of-content parsing#10039
douzzer merged 2 commits intowolfSSL:masterfrom
anhu:pkcs7_oob

Conversation

@anhu
Copy link
Copy Markdown
Member

@anhu anhu commented Mar 21, 2026

Fixes ZD 21399 Finding #1

@anhu anhu requested a review from wolfSSL-Bot March 21, 2026 01:35
@anhu anhu self-assigned this Mar 21, 2026
@anhu
Copy link
Copy Markdown
Member Author

anhu commented Mar 21, 2026

Jenkins retest this please.

Copy link
Copy Markdown
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Clang-tidy: unchecked XFSEEK return values in test_pkcs7.c lines 4878 and 4880 — the PR adds calls like XFSEEK(f, 0, XSEEK_END); and XFSEEK(f, 0, XSEEK_SET); without using the return value, triggering bugprone-unused-return-value. This failed 3 of 4 clang-tidy configurations (PRB-multi-test-script #9983).

Fix: capture and check the return value, e.g.:

if (XFSEEK(f, 0, XSEEK_END) != 0) { /* handle error */ }

@jeremylaratro
Copy link
Copy Markdown

Hi! I submitted the initial ticket. Would using something like “ ExpectIntEQ” work?
Ex.

    ExpectTrue((f = XFOPEN(fName, "rb")) != XBADFILE);
    if (f != XBADFILE) {
        ExpectIntEQ(XFSEEK(f, 0, XSEEK_END), 0);
        ExpectIntGT(derSz = (word32)XFTELL(f), 0);
        ExpectIntEQ(XFSEEK(f, 0, XSEEK_SET), 0);
        ExpectNotNull(der = (byte*)XMALLOC(derSz, HEAP_HINT,
                                           DYNAMIC_TYPE_TMP_BUFFER));
        if (der != NULL) {
            ExpectIntEQ((int)XFREAD(der, 1, derSz, f), (int)derSz);
        }
        XFCLOSE(f);
    }

@anhu
Copy link
Copy Markdown
Member Author

anhu commented Mar 23, 2026

Hi! I submitted the initial ticket. Would using something like “ ExpectIntEQ” work? Ex.

Yes, I believe so. Sorry it has taken so long for me to reply. Its been a busy weekend. :)

@jeremylaratro
Copy link
Copy Markdown

Hi! I submitted the initial ticket. Would using something like “ ExpectIntEQ” work? Ex.

Yes, I believe so. Sorry it has taken so long for me to reply. Its been a busy weekend. :)

No worries at all. You've been very fast to reply, take as much time as you need, happy to help!

@dgarske
Copy link
Copy Markdown
Contributor

dgarske commented Mar 24, 2026

Jenkins retest this please. @anhu Guessing the new file is not in an include.am which is why the make distcheck is failing.

@anhu anhu assigned wolfSSL-Bot and unassigned anhu Apr 1, 2026
@anhu anhu added the For This Release Release version 5.9.1 label Apr 1, 2026
@anhu
Copy link
Copy Markdown
Member Author

anhu commented Apr 1, 2026

All checks passed. Good to go.

@anhu anhu requested a review from dgarske April 1, 2026 18:29
@anhu
Copy link
Copy Markdown
Member Author

anhu commented Apr 1, 2026

All tests passed. Ready for merge.

@douzzer douzzer added the Staged Staged for merge pending final test results and review label Apr 1, 2026
@douzzer douzzer merged commit 49cbbab into wolfSSL:master Apr 2, 2026
649 of 650 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.1 Staged Staged for merge pending final test results and review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants