[PATCH 1/2] sha1_loose_object_info: return error for corrupted objects
- From
Jeff King <peff@peff.net>
- Date
- Apr 1, 2017, 08:05 UTC
- Message-ID
- <20170401080521.ehjc6jomjd5py3lj@sigill.intra.peff.net>
- In-Reply-To
- <20170401080349.lccextuc3l6fgs6j@sigill.intra.peff.net>
When sha1_loose_object_info() finds that a loose object file cannot be stat(2)ed or mmap(2)ed, it returns -1 to signal an error to the caller. However, if it found that the loose object file is corrupt and the object data cannot be used from it, it stuffs OBJ_BAD into "type" field of the object_info, but returns zero (i.e., success), which can confuse callers.
This is due to 052fe5eac (sha1_loose_object_info: make type lookup optional, 2013-07-12), which switched the return to a strict success/error, rather than returning the type (but botched the return).
Callers of regular sha1_object_info() don't notice the difference, as that function returns the type (which is OBJ_BAD in this case). However, direct callers of sha1_object_info_extended() see the function return success, but without setting any meaningful values in the object_info struct, leading them to access potentially uninitialized memory.
The easiest way to see the bug is via "cat-file -s", which will happily ignore the corruption and report whatever value happened to be in the "size" variable.
Signed-off-by: Jeff King <peff@peff.net> --- So not only does it not fail, but running with --valgrind complains.
sha1_file.c | 2 +- t/t1060-object-corruption.sh | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c index 71063890f..368c89b5c 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2952,7 +2952,7 @@ static int sha1_loose_object_info(const unsigned char *sha1, if (status && oi->typep) *oi->typep = status; strbuf_release(&hdrbuf); - return 0; + return (status < 0) ? status : 0; } int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, unsigned flags) diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh index 3f8705139..3a88d79c5 100755 --- a/t/t1060-object-corruption.sh +++ b/t/t1060-object-corruption.sh @@ -53,6 +53,13 @@ test_expect_success 'streaming a corrupt blob fails' ' ) ' +test_expect_success 'getting type of a corrupt blob fails' ' + ( + cd bit-error && + test_must_fail git cat-file -s HEAD:content.t + ) +' + test_expect_success 'read-tree -u detects bit-errors in blobs' ' ( cd bit-error &&
-- 2.12.2.943.g91cb50fd8