-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
strncpy out of bounds fix #1785
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
welp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the off-by-one problem. I think we need to make one change to how the string is copied over, then this is good to go
while (i <= len) { | ||
char current = src[i]; | ||
dst[i] = current; | ||
while (i < destination_len && source[i] != '\0') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change means the null termination character is not copied to the destination array, which could cause various problems downstream as lots of C code assumes that strings must have that character.
e537269
to
717f942
Compare
99038bb
to
cc4fccb
Compare
We use strncpy in many places. In almost all places, we call it with the size of the destination array. That means something like this:
In that case, sizeof(destinationArray) will be 3. That means the while loop will execute until 4, and we will try to write
src[3]
todst[3]
, which is out of bounds.While C might let you write out of bounds, it may cause memory corruption or failure.
Changes:
i
won't be equal todestination_len
.source[i] != '\0'
verification in case the source is smaller than the destination. Strings passed in src should end in that termination char. If they don't, we might face another OOB. We could solve this by also passing source_len as a parameter.