Fix buffer overflow in ut_generate() when truncating vlist#3
Open
jaredmauch wants to merge 1 commit intofutatuki:mainfrom
Open
Fix buffer overflow in ut_generate() when truncating vlist#3jaredmauch wants to merge 1 commit intofutatuki:mainfrom
jaredmauch wants to merge 1 commit intofutatuki:mainfrom
Conversation
The vlistlen variable was calculated before pointer p was incremented multiple times in the switch statement. When strdup(p) was called, the actual string length was shorter than vlistlen, causing a potential buffer overflow when accessing vlist[vlistlen - 1]. Fix by recalculating the actual length from the current p position to the closing brace (eb) after all pointer increments, ensuring we never access beyond the allocated string bounds. Also add NULL check for strdup() failure case. Fixes buffer overflow vulnerability in URI template processing.
futatuki
reviewed
Dec 23, 2025
Comment on lines
+755
to
+762
| /* Recalculate length from current p position to closing brace */ | ||
| /* to avoid buffer overflow from using stale vlistlen value */ | ||
| if (eb > p) | ||
| { | ||
| size_t actual_len = eb - p; | ||
| if (actual_len < strlen(vlist)) | ||
| vlist[actual_len] = '\0'; | ||
| } |
Owner
There was a problem hiding this comment.
It seems that NULL check for p is needed (and using UT_ERROR_MALFORMED here is okey because there is no other value to indicate for errors).
However does the calculation for acutal_len here really need? As far as I read the spec of strdup(), it returns NULL if insufficient memory is available (that is, copied string never is truncated, it should be copied whole string or NULL is returned, I think).
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.
The vlistlen variable was calculated before pointer p was incremented multiple times in the switch statement. When strdup(p) was called, the actual string length was shorter than vlistlen, causing a potential buffer overflow when accessing vlist[vlistlen - 1].
Fix by recalculating the actual length from the current p position to the closing brace (eb) after all pointer increments, ensuring we never access beyond the allocated string bounds.
Also add NULL check for strdup() failure case.
Fixes buffer overflow vulnerability in URI template processing.