Fix truncation of SHA in error message for git_odb_read by meatcoder · Pull Request #3818 · libgit2/libgit2 · GitHub
Skip to content

Fix truncation of SHA in error message for git_odb_read#3818

Closed
meatcoder wants to merge 3 commits into
libgit2:masterfrom
meatcoder:fix_odb_read_error
Closed

Fix truncation of SHA in error message for git_odb_read#3818
meatcoder wants to merge 3 commits into
libgit2:masterfrom
meatcoder:fix_odb_read_error

Conversation

@meatcoder

Copy link
Copy Markdown

This is an attempt to fix #3753. The bug appears to only occur when the oid_len parameter passed to git_odb__error_notfound() is the maximum length of GIT_OID_HEXSZ. This PR fixes it by passing the actual buffer length to git_oid_tostr().

@meatcoder

Copy link
Copy Markdown
Author

@ethomson

Copy link
Copy Markdown
Member

Welcome @meatcoder ! That's for looking into this;

Since git_oid_tostr takes the length of the buffer to write, I think that the function (as it exists in master) will truncate all ID strings, not just those of length GIT_OID_HEXSZ. I think that your patch will stop truncating the ID, but only when it is of length GIT_OID_HEXSZ. If someone were to provide (say) an id fragment that was 7 bytes long (when converted to hexadigits) then we would call git_oid_tostr(oidstr, 7, oid).

git_oid_tostr is documented to take the second argument as the buffer length, and is guaranteed to write a NULL terminator. This means that it would only write the first 6 characters, to make room for that NULL.

So I think that while this fixes the common case (when the ID is of length GIT_OID_HEXSZ) this does not fix all cases. Would you mind amending this to take care of all cases? Thanks!

@meatcoder

Copy link
Copy Markdown
Author

Thanks for looking at this and the detailed explanation @ethomson. I've updated the pull request to to also fix the cases where shorter ID strings are passed to git_odb_read(). Let me know how it looks. Thanks again.

@ethomson

Copy link
Copy Markdown
Member

This looks great, thanks @meatcoder !

ethomson pushed a commit that referenced this pull request Jun 20, 2016
Fix truncation of SHA in error message for git_odb_read
@ethomson

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

git_odb_read is giving error with off-by-1 sha string

2 participants