sscanf solution with buffer overflow prevention #4438
Conversation
|
This pull request didn't trigger Jenkins as its author isn't in the allow list. An organization member must perform one of the following:
Those commands are simple Github comments of the format: "jenkins: COMMAND" |
|
I saw that the sanitizer fails because of a warning produced by the string literal fmt_str. We make fmt_str a string literal by using [] rather than dynamically allocating, but we still get this error. Should we still change our code to avoid the error? |
|
Yeah, pretty much everyone runs with those warnings enabled in production so that needs to be addressed. |
|
Is the check for the size of PATH necessary before we read in the buffer? |
|
Should I squash the latest commits to close the sscanf issue? |
|
Probably good to squash. I'm really not sure about the fix though but I'm also not a C developer so I'll let @mihalicyn, @brauner or @hallyn look at it. |
First checks if PATH_MAX 4096 bytes Signed-off-by: Devon Schwartz <devon.s.schwartz@utexas.edu>
DevonSchwartz
force-pushed
the
fix_sscanf_issue
branch
from
88b9480
to
a4193c3
Compare
|
I don't understand this. We are not using |
This fix is related to issue #4366.
This issue points to unsafe call to sscanf to get a username in the case of an invalid uid. The sscanf call has a potential buffer overflow. However, this sscanf call is not implimented in main. We implement the sscanf call safely in main by limiting user input to the size of PATH_MAX.
This commit creates a warning that fmt_str is not a string literal. However, fmt_str is equal to "%[PATH_MAX]s"
fix #4366