Skip to content

Fixes in SP int and DH#10478

Merged
SparkiDev merged 2 commits into
wolfSSL:masterfrom
embhorn:zd21821
May 27, 2026
Merged

Fixes in SP int and DH#10478
SparkiDev merged 2 commits into
wolfSSL:masterfrom
embhorn:zd21821

Conversation

@embhorn

@embhorn embhorn commented May 13, 2026

Copy link
Copy Markdown
Member

Description

  • Fix SP int truncation issue
  • Check params to GeneratePrivateDh186

Thanks to Kr0emer for reporting these issues and testing the fixes.

Fixes:

  • zd21821
  • zd21822

Testing

Added regression test to dh_fips_generate_test

Checklist

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

@embhorn embhorn self-assigned this May 13, 2026
@embhorn embhorn changed the title Zd21821 Fixes in SP int and DH May 13, 2026

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10478

Scan targets checked: wolfcrypt-bugs, wolfcrypt-src

No new issues found in the changed files. ✅

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes integer truncation issues in sp_int bit/digit shift APIs and adds an upper-bound check on *privSz in GeneratePrivateDh186 to prevent stack buffer overflow. Adds a regression test for the DH case.

Changes:

  • Widen intermediate computations in sp_set_bit, sp_rshd, sp_rshb, and sp_mod_2d to int to avoid sp_size_t (potentially word16) wrap-around before bounds checks.
  • Reject *privSz > DH_MAX_SIZE in GeneratePrivateDh186 before writing into the stack cBuf.
  • Add a regression test in dh_fips_generate_test exercising the oversize privSz rejection path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
wolfcrypt/src/sp_int.c Avoid narrowing of shift/digit counts to sp_size_t before bounds checks.
wolfcrypt/src/dh.c Add *privSz > DH_MAX_SIZE guard in GeneratePrivateDh186.
wolfcrypt/test/test.c Regression test invoking wc_DhGenerateKeyPair with oversized *privSz.
Comments suppressed due to low confidence (1)

wolfcrypt/test/test.c:1

  • The regression test only covers the case *privSz == DH_MAX_SIZE + 1. It would be worth also exercising a value comfortably above any plausible byte interpretation (e.g. DH_MAX_SIZE * 2 or 0xFFFFFFFF - 8) and a boundary value at exactly DH_MAX_SIZE to lock down the accepted/rejected boundary, especially given the bit-vs-byte ambiguity in the new bound.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wolfcrypt/src/dh.c
Comment thread wolfcrypt/src/dh.c
Comment thread wolfcrypt/src/sp_int.c
Comment thread wolfcrypt/src/sp_int.c
@embhorn

embhorn commented May 13, 2026

Copy link
Copy Markdown
Member Author

Will rebase after #10480 gets merged

@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m4-baremetal

@embhorn

embhorn commented May 14, 2026

Copy link
Copy Markdown
Member Author

Jenkins retest this please

@SparkiDev SparkiDev merged commit 7bcc613 into wolfSSL:master May 27, 2026
452 of 453 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants