Skip to content

Defensive code additions for sanity checks on input arguments with Base64, PEM write, mp_read_unsigned_bin #10721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JacobBarthelmeh wants to merge 3 commits into
base: master
Choose a base branch
from
+24 −5
Open

Defensive code additions for sanity checks on input arguments with Base64, PEM write, mp_read_unsigned_bin #10721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01df001
sanity check on read unsigned bin length argument
JacobBarthelmeh Jun 17, 2026
a9ab066
adjustment to sanity checks for avoiding potential overflow
JacobBarthelmeh Jun 17, 2026
0ed2e10
hardening against potential overflow cases
JacobBarthelmeh Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/pk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6911,9 +6911,15 @@ static int pem_write_data(const char *name, const char *header,
int headerLen;
char* pem = NULL;
word32 pemLen;
word32 derLen = (word32)len;
word32 derLen;
byte* p;

/* Reject lengths that would wrap the PEM size calculation below. */
if ((len < 0) || ((word32)len >= (WOLFSSL_MAX_32BIT / 4))) {
return BAD_FUNC_ARG;
}
derLen = (word32)len;
Comment on lines +6917 to +6921

nameLen = (int)XSTRLEN(name);
headerLen = (int)XSTRLEN(header);

Expand Down
8 changes: 4 additions & 4 deletions wolfcrypt/src/asn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2461,8 +2461,8 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
*len = 0;

/* Check there is at least one byte available containing length information.
*/
if ((idx + 1) > maxIdx) {
* Use >= to avoid a word32 wrap when idx is near UINT_MAX. */
if (idx >= maxIdx) {
WOLFSSL_MSG("GetLength - bad index on input");
return BUFFER_E;
}
Expand Down Expand Up @@ -2495,7 +2495,7 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
}

/* Check the number of bytes required are available. */
if ((idx + (word32)bytes) > maxIdx) {
if ((word32)bytes > (maxIdx - idx)) {
WOLFSSL_MSG("GetLength - bad long length");
return BUFFER_E;
}
Expand All @@ -2520,7 +2520,7 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
}

/* When requested, check the buffer has at least length bytes left. */
if (check && ((idx + length) > maxIdx)) {
if (check && (length > (maxIdx - idx))) {
WOLFSSL_MSG("GetLength - value exceeds buffer length");
return BUFFER_E;
}
Expand Down
4 changes: 4 additions & 0 deletions wolfcrypt/src/coding.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,10 @@ static int DoBase64_Encode(const byte* in, word32 inLen, byte* out,
if (in == NULL && inLen > 0)
return BAD_FUNC_ARG;

/* Reject lengths that would wrap the encoded-size calculation below. */
if (inLen >= (WOLFSSL_MAX_32BIT / 4))
return BAD_FUNC_ARG;

outSz = (inLen + 3 - 1) / 3 * 4;
addSz = (outSz + BASE64_LINE_SZ - 1) / BASE64_LINE_SZ; /* new lines */
Comment on lines +486 to 491

Expand Down
9 changes: 9 additions & 0 deletions wolfcrypt/src/integer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,11 +762,20 @@ int mp_read_unsigned_bin (mp_int * a, const unsigned char *b, int c)
int res;
int digits_needed;

if (c < 0) {
return MP_VAL;
}

while (c > 0 && b[0] == 0) {
c--;
b++;
}

/* reject sizes where the bit count would overflow an int */
if (c > (WOLFSSL_MAX_32BIT - (DIGIT_BIT - 1)) / CHAR_BIT) {

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [Medium] mp_read_unsigned_bin overflow guard uses unsigned max for signed int math · Incorrect sizeof/type usage

The guard bounds c by WOLFSSL_MAX_32BIT (0xffffffff), but c, CHAR_BIT and digits_needed are signed int. Values of c in (INT_MAX/8, UINT_MAX/8] pass the check yet still overflow c * CHAR_BIT in digits_needed, the exact signed-int overflow the check claims to prevent.

Fix: Bound c against INT_MAX instead of WOLFSSL_MAX_32BIT: c > (INT_MAX - (DIGIT_BIT - 1)) / CHAR_BIT.

return MP_VAL;
}

digits_needed = ((c * CHAR_BIT) + DIGIT_BIT - 1) / DIGIT_BIT;
Comment on lines +774 to 779

/* make sure there are enough digits available */
Expand Down
Loading