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
When the runtime is invoked, it copies (strcpy) argv[0] or the TARGET_APPIMAGE environemnt variable into a buffer that has a dimension of PATH_MAX, and same thing with the TMPDIR env variable.
If the values saved in argv[0] or those environemnt variables are longer than PATH_MAX, the runtime currently segfaults due to buffer overflow.
This patch introduces a simple check that exits with an error message if that happens instead of causing a segfault. This is obtained by changing strcpy to memccpy, a POSIX standard function that makes it easy to detect truncation (check if the return value is NULL).
Could the code be simplified? Maybe we could avoid redundant calls to getenv and the strcpy before memccpy. Additionally, modern C suggests using snprintf instead of strcpy and memccpy for safer and more concise string handling. What do you think?
Could the code be simplified? Maybe we could avoid redundant calls to getenv and the strcpy before memccpy.
Right, I extracted a variable to avoid repeated getenv("TARGET_APPIMAGE") calls and initialized argv0_path so we don't have to call strcpy to copy "/proc/self/exe"
Additionally, modern C suggests using snprintf instead of strcpy and memccpy for safer and more concise string handling. What do you think?
I do not have a strong preference towards one or the other, I believe them to be about the same in this context in terms of conciseness and safety.
I initially chose memccpy because there is no "formatting" involved and because we can check if the string is too big for the buffer with a simple NULL check on the result.
As for safety, I believe memccpy to be as safe as snprintf since both functions stop copying data after the buffer length we provide.
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.
When the runtime is invoked, it copies (strcpy) argv[0] or the TARGET_APPIMAGE environemnt variable into a buffer that has a dimension of PATH_MAX, and same thing with the TMPDIR env variable.
If the values saved in argv[0] or those environemnt variables are longer than PATH_MAX, the runtime currently segfaults due to buffer overflow.
This patch introduces a simple check that exits with an error message if that happens instead of causing a segfault. This is obtained by changing strcpy to memccpy, a POSIX standard function that makes it easy to detect truncation (check if the return value is NULL).