-
Notifications
You must be signed in to change notification settings - Fork 11
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
base: priettt/ndkCrashFramesFix
Are you sure you want to change the base?
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.
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.