You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This patch addresses a stack-based buffer overflow in the steamclient_dos_to_unix_path function.
The original implementation used strcpy to copy the src string into a fixed-size stack buffer (char buffer[4096]) without bounds checking. This could lead to buffer overflow if the input string exceeds the buffer size, potentially causing crashes or unexpected behavior.
This fix replaces the unsafe strcpy call with strncpy, and ensures null-termination by explicitly setting the last byte of the buffer to \0. This change mitigates the overflow risk while preserving the original logic of the function.
Summary of changes:
Replaced strcpy(dst, src) with strncpy(dst, src, sizeof(buffer) - 1)
Added dst[sizeof(buffer) - 1] = '\0' to guarantee null-termination
If the array pointed to by s2 is a string that is shorter than n
bytes, NUL characters shall be appended to the copy in the array
pointed to by s1, until n bytes in all are written.
This will copy unnecessary NUL characters if src is shorter than sizeof(buffer) - 1
If we look at strncat:
This function appends at most ssize non-null bytes from the array
pointed to by src, followed by a null character, to the end of the
string pointed to by dst.
strncat will not copy unnecessary NUL characters.
Per the Stack Overflow link below, the fix could look like:
If the array pointed to by s2 is a string that is shorter than n
bytes, NUL characters shall be appended to the copy in the array
pointed to by s1, until n bytes in all are written.
This will copy unnecessary NUL characters if src is shorter than sizeof(buffer) - 1 If we look at strncat:
This function appends at most ssize non-null bytes from the array
pointed to by src, followed by a null character, to the end of the
string pointed to by dst.
strncat will not copy unnecessary NUL characters.
Per the Stack Overflow link below, the fix could look like:
Good point about strncat — makes total sense and yeah, definitely cleaner than strncpy in this case. I’ve updated the patch to use that instead. Appreciate the suggestion!
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
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.
This patch addresses a stack-based buffer overflow in the
steamclient_dos_to_unix_pathfunction.The original implementation used
strcpyto copy thesrcstring into a fixed-size stack buffer (char buffer[4096]) without bounds checking. This could lead to buffer overflow if the input string exceeds the buffer size, potentially causing crashes or unexpected behavior.This fix replaces the unsafe
strcpycall withstrncpy, and ensures null-termination by explicitly setting the last byte of the buffer to\0. This change mitigates the overflow risk while preserving the original logic of the function.Summary of changes:
strcpy(dst, src)withstrncpy(dst, src, sizeof(buffer) - 1)dst[sizeof(buffer) - 1] = '\0'to guarantee null-termination