mirror of
https://github.com/libgit2/libgit2.git
synced 2026-06-22 06:26:26 +00:00
fix(sha256): thread-safety bug in builtin SHA-256
The implementation here seems to be sort of a copy
from the reference impl in RFC 6234 [2].
When multiple threads hash concurrently,
they race on this shared static variable.
It then corrupts the length-overflow detection,
and produces incorrect SHA-256 digests.
Here we replace it with a `static` function with a local variable.
The bug only affects the `GIT_SHA256_BUILTIN` backend.
The SHA-1 code path uses `sha1dc` which does not have this issue.
Reproducer:
```c
#include <stddef.h>
#include <stdio.h>
#include <string.h>
#include <pthread.h>
#include <git2.h>
#define NUM_THREADS 8
#define ITERATIONS 100000
static volatile int found_bug = 0;
void *hash_thread(void *arg) {
int id = *(int *)arg;
const char *data = "hello world\n";
size_t len = strlen(data);
git_object_id_options opts = GIT_OBJECT_ID_OPTIONS_INIT;
opts.object_type = GIT_OBJECT_BLOB;
opts.oid_type = GIT_OID_SHA256;
git_oid reference, result;
git_object_id_from_buffer(&reference, data, len, &opts);
for (int i = 0; i < ITERATIONS && !found_bug; i++) {
git_object_id_from_buffer(&result, data, len, &opts);
if (!git_oid_equal(&reference, &result)) {
found_bug = 1;
printf("BUG: thread %d, iteration %d\n", id, i);
break;
}
}
return NULL;
}
int main(void) {
git_libgit2_init();
pthread_t threads[NUM_THREADS];
int ids[NUM_THREADS];
for (int i = 0; i < NUM_THREADS; i++) {
ids[i] = i;
pthread_create(&threads[i], NULL, hash_thread, &ids[i]);
}
for (int i = 0; i < NUM_THREADS; i++)
pthread_join(threads[i], NULL);
if (!found_bug)
printf("No bug triggered\n");
git_libgit2_shutdown();
return found_bug ? 1 : 0;
}
```
Build and run (from libgit2 repo root):
```sh
mkdir build && cd build
cmake .. -DEXPERIMENTAL_SHA256=ON -DUSE_SHA256=Builtin \
-DUSE_HTTPS=OFF -DUSE_SSH=OFF -DUSE_NTLMCLIENT=OFF \
-DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Debug
make libgit2package
cd ..
cc -O0 -pthread -DGIT_EXPERIMENTAL_SHA256=1 \
-I include -o repro repro.c \
build/libgit2-experimental.a -lz -lpcre2-8
./repro
```
See <https://github.com/rust-lang/git2-rs/issues/1255> for more.
[1]: https://github.com/libgit2/libgit2/blob/1affb8b19/src/util/hash/rfc6234/sha224-256.c#L86-L91
[2]: https://www.rfc-editor.org/rfc/rfc6234#section-8.2.2
This commit is contained in:
committed by
Edward Thomson
parent
4d232e344d
commit
e2e2cfb006
@@ -83,12 +83,14 @@
|
||||
* Add "length" to the length.
|
||||
* Set Corrupted when overflow has occurred.
|
||||
*/
|
||||
static uint32_t addTemp;
|
||||
#define SHA224_256AddLength(context, length) \
|
||||
(addTemp = (context)->Length_Low, (context)->Corrupted = \
|
||||
(((context)->Length_Low += (length)) < addTemp) && \
|
||||
(++(context)->Length_High == 0) ? shaInputTooLong : \
|
||||
(context)->Corrupted )
|
||||
static int SHA224_256AddLength(SHA256Context *context, unsigned int length)
|
||||
{
|
||||
uint32_t addTemp = context->Length_Low;
|
||||
context->Length_Low += length;
|
||||
if (context->Length_Low < addTemp && ++context->Length_High == 0)
|
||||
context->Corrupted = shaInputTooLong;
|
||||
return context->Corrupted;
|
||||
}
|
||||
|
||||
/* Local Function Prototypes */
|
||||
static int SHA224_256Reset(SHA256Context *context, uint32_t *H0);
|
||||
|
||||
Reference in New Issue
Block a user