hash: openssl: check return values of SHA1_* functions - #4437
Merged
Conversation
Member
|
Agreed. We should set an error message though, otherwise it will likely be sent empty down to the caller, which would be unhelpful. |
The OpenSSL functions `SHA1_Init`, `SHA1_Update` and `SHA1_Final` all return 1 for success and 0 otherwise, but we never check their return values. Do so.
pks-t
force-pushed
the
pks/openssl-hash-errors
branch
from
January 3, 2018 12:49
ffcbdbe to
75e1737
Compare
The function `ERR_error_string` can be invoked without providing a buffer, in which case OpenSSL will simply return a string printed into a static buffer. Obviously and as documented in ERR_error_string(3), this is not thread-safe at all. As libgit2 is a library, though, it is easily possible that other threads may be using OpenSSL at the same time, which might lead to clobbered error strings. Fix the issue by instead using a stack-allocated buffer. According to the documentation, the caller has to provide a buffer of at least 256 bytes of size. While we do so, make sure that the buffer will never get overflown by switching to `ERR_error_string_n` to specify the buffer's size.
Member
Author
|
Thanks @ethomson. I am now also setting an error message, even though I'm not using 'ERR_error_message' as I have no idea whether that is being set for SHA1 errors. While digging into its documentation, I also realized we are using 'ERR_error_message' in a thread-unsafe way, as we have it print into a shared static buffer. I fixed that issue as well. |
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The OpenSSL functions
SHA1_Init,SHA1_UpdateandSHA1_Finalallreturn 1 for success and 0 otherwise, but we never check their return
values. Do so.