iperf_auth: fix heap-buffer-overflow in Base64Decode#2027
Open
JustinStitt wants to merge 1 commit intoesnet:masterfrom
Open
iperf_auth: fix heap-buffer-overflow in Base64Decode#2027JustinStitt wants to merge 1 commit intoesnet:masterfrom
JustinStitt wants to merge 1 commit intoesnet:masterfrom
Conversation
A heap-buffer-overflow was discovered in `Base64Decode` when processing
malformed input, such as a single '=' character.
The issue stemmed from an unsigned integer underflow in
`calcDecodeLength`. For an input of length 1 with 1 padding character,
the formula `(len*3)/4 - padding` resulted in `0 - 1`, producing
`SIZE_MAX`. In `Base64Decode`, this value was truncated when assigned to
an `int decodeLen`, resulting in `-1`. This caused `malloc(decodeLen +
1)` to call `malloc(0)` and a subsequent out-of-bounds write at
`(*buffer)[-1]`.
Changes:
- Modified `calcDecodeLength` to explicitly check for underflow and
return 0 if padding exceeds the calculated base length.
- Changed the type of `decodeLen` from `int` to `size_t` in
`Base64Decode` to ensure consistency and avoid signedness issues.
- Added a NULL pointer check for the `malloc` allocation.
Full summary: The vulnerability was a heap-buffer-overflow in
`Base64Decode` in `/src/iperf/src/iperf_auth.c`.
Root Cause: The helper function `calcDecodeLength` calculates the
decoded length of a Base64 string using the formula:
return (len*3)/4 - padding;
where `len` is the input string length and `padding` is the number of
'=' characters at the end (1 or 2).
When the input is a single '=' character:
- `len` is 1.
- `padding` is 1.
- `(len*3)/4` is 0.
- `0 - 1` results in an unsigned integer underflow on `size_t`, producing `SIZE_MAX`.
In `Base64Decode`:
int decodeLen = calcDecodeLength(b64message);
*buffer = (unsigned char*)malloc(decodeLen + 1);
(*buffer)[decodeLen] = '\0';
The `SIZE_MAX` returned by `calcDecodeLength` is assigned to `int
decodeLen`, which casts it to `-1`. `malloc(decodeLen + 1)` becomes
`malloc(0)`, allocating a minimal chunk (1 byte). `(*buffer)[decodeLen]
= '\0'` becomes `(*buffer)[-1] = '\0'`, writing 1 byte before the
allocated buffer.
Debugger verification: Before the fix, the debugger showed `decodeLen`
as `-1` (int) and ASAN reported a write 1 byte before the allocated
region. After the fix, `decodeLen` is `0`, and the program runs without
error.
Fix: The fix involves:
1. Modifying `calcDecodeLength` to explicitly check if `padding >
(len*3)/4` and return 0 to prevent underflow.
2. Changing `decodeLen` to `size_t` in `Base64Decode`.
3. Adding a NULL check for `malloc`.
Co-authored-by: CodeMender <codemender-patching@google.com>
Reviewed-by: Meder Kydyraliev <meder@google.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Fixes: https://issues.oss-fuzz.com/issues/474401004
Contributor
|
Hi Justin, We responded to an email from David (David@adalogics.com) about a possible disclosure on Friday, February 27th, 2026, but never heard a response about the details or a patch. The email we normally use for security disclosures is iperf@es.net. Thank you for the pull request. We'll review the patch soon. Thanks, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OSSFuzz initially reported iperf:auth_fuzzer: Heap-buffer-overflow in Base64Decode
earlier this year. The disclosure deadline for that particular report has passed and as such I figured I'd post the report again as well as a patch (this PR).
AI DISCLAIMER: We ran CodeMender against the report in an agent+fuzzer feedback loop until we arrived at this simple patch which was then reviewed by several humans here at Google. If this patch 1) doesn't meet iperf's coding standards or 2) is not actually correct then please let me know and I (human) will fix it :)
calcDecodeLengthwas susceptible to an unsigned integer underflow when processing malformed Base64 input (e.g., a single '=' character). In such cases, the calculation(len*3)/4 - paddingresulted inSIZE_MAX.In
Base64Decode, thisSIZE_MAXvalue was assigned to a signedint, resulting in-1. This led tomalloc(0)(allocating a minimal chunk) followed by an out-of-bounds write at(*buffer)[-1].This patch addresses the issue by:
calcDecodeLengthto return 0 if the padding exceeds the calculated base length, preventing underflow.decodeLenvariable type frominttosize_tto prevent signedness issues.mallocallocation.Verified with ASAN: previously reported a 1-byte write before the allocated region; now runs without error.
Full summary:
The vulnerability was a heap-buffer-overflow in
Base64Decodein/src/iperf/src/iperf_auth.c.Root Cause:
The helper function
calcDecodeLengthcalculates the decoded length of a Base64 string using the formula:where
lenis the input string length andpaddingis the number of '=' characters at the end (1 or 2).When the input is a single '=' character:
lenis 1.paddingis 1.(len*3)/4is 0.0 - 1results in an unsigned integer underflow onsize_t, producingSIZE_MAX.In
Base64Decode:The
SIZE_MAXreturned bycalcDecodeLengthis assigned toint decodeLen, which casts it to-1.malloc(decodeLen + 1)becomesmalloc(0), allocating a minimal chunk (1 byte).(*buffer)[decodeLen] = '\0'becomes(*buffer)[-1] = '\0', writing 1 byte before the allocated buffer.<snip>
Co-authored-by: CodeMender codemender-patching@google.com
Reviewed-by: Meder Kydyraliev meder@google.com
Fixes: https://issues.oss-fuzz.com/issues/474401004