Skip to content

fix(sha256): thread-safety bug in builtin SHA-256#7266

Merged
ethomson merged 1 commit into
libgit2:mainfrom
weihanglo:fix
May 16, 2026
Merged

fix(sha256): thread-safety bug in builtin SHA-256#7266
ethomson merged 1 commit into
libgit2:mainfrom
weihanglo:fix

Conversation

@weihanglo
Copy link
Copy Markdown
Contributor

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:

#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):

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 rust-lang/git2-rs#1255 for more.

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 <rust-lang/git2-rs#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
@ethomson
Copy link
Copy Markdown
Member

Woof, good catch. Thanks for the fix.

@ethomson ethomson merged commit bc1ab28 into libgit2:main May 16, 2026
22 checks passed
@weihanglo weihanglo deleted the fix branch May 16, 2026 10:12
@weihanglo
Copy link
Copy Markdown
Contributor Author

Thanks for the swift review!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants