Skip to content

Fix buffer overflow in ut_generate() when truncating vlist#3

Open
jaredmauch wants to merge 1 commit intofutatuki:mainfrom
jaredmauch:fix/libut-buffer-overflow-vlistlen
Open

Fix buffer overflow in ut_generate() when truncating vlist#3
jaredmauch wants to merge 1 commit intofutatuki:mainfrom
jaredmauch:fix/libut-buffer-overflow-vlistlen

Conversation

@jaredmauch
Copy link

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.

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.
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';
}
Copy link
Owner

Choose a reason for hiding this comment

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

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).

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.

2 participants