In 10a6762719 (object-file: adapt `stream_object_signature()` to take a stream, 2026-02-23), we have refactored `stream_object_signature()` so that it doesn't create the stream ad-hoc anymore. Instead, callers are expected to pass in a stream, which allows them to construct the streams from different sources.
While the stream was previously managed by `stream_object_signature()`, the full lifecycle is now owned by the caller. Hence, it's the caller's responsibility to close the stream, and the called function shouldn't do that anymore.
And while the mentioned commit did drop one call that closed the stream, there's a second such call that was missed when reading from the stream fails. The consequence of this can be a double free of the stream.
Fix the bug by dropping that leftover call to `odb_read_stream_close()`.
Note that it was originally discussed whether this should be treated as a security vulnerability. But there are only two callers: once via `parse_object_with_flags()`, and once via `verify_packfile()`. Neither of these callers plays any role on the transport layer, so this issue is only relevant for objects that are already available via the local object database. Furthermore, a packfile that is corrupted in this way would be detected when receiving the packfile, so it's not easy for an adversary to plant such a packfile, either. Consequently, we decided that this is not covered as part of our threat model.
Reported-by: xuqing yang <rigelyoung@icloud.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Hi,
this patch fixes a double-free of object streams introduced via 10a6762719 (object-file: adapt `stream_object_signature()` to take a stream, 2026-02-23). It was reported to the security mailing list, but because we couldn't find a way to abuse this issue remotely we decided that the issue can be fixed in the open.
The fix is built on top of v2.54.0, which is where this issue was introduced. It merges cleanly to "master".
Thanks!
Patrick --- object-file.c | 5 +---- t/t1450-fsck.sh | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-)
Show changes to 2 files +18 −4
object-file.c, t/t1450-fsck.sh
diff --git a/object-file.c b/object-file.c index 2acc9522df..610faba5b6 100644 --- a/object-file.c +++ b/object-file.c @@ -150,11 +150,8 @@ int stream_object_signature(struct repository *r, for (;;) { char buf[1024 * 16]; ssize_t readlen = odb_read_stream_read(st, buf, sizeof(buf)); - - if (readlen < 0) { - odb_read_stream_close(st); + if (readlen < 0) return -1; - } if (!readlen) break; git_hash_update(&c, buf, readlen); diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 54e81c2636..bc326a78f6 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -538,6 +538,23 @@ test_expect_success 'rev-list --verify-objects with bad sha1' ' test_grep -q "error: hash mismatch $(dirname $new)$(test_oid ff_2)" out ' +test_expect_success 'rev-list --verify-objects with truncated loose blob' ' + git init truncated-blob && + ( + cd truncated-blob && + blob=$(test-tool genrandom one 5k | git hash-object -t blob -w --stdin) && + obj=.git/objects/$(test_oid_to_path $blob) && + + # Truncate the loose blob such that its header can still be + # parsed, but reading the object data fails mid-stream. + test_copy_bytes 64 <"$obj" >obj.tmp && + mv obj.tmp "$obj" && + + test_must_fail git rev-list --verify-objects "$blob" 2>err && + test_grep "hash mismatch" err + ) +' + # An actual bit corruption is more likely than swapped commits, but # this provides an easy way to have commits which don't match their purported # hashes, but which aren't so broken we can't read them at all. --- base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0 change-id: 20260710-pks-odb-stream-double-close-49c4b4a93f01