summaryrefslogtreecommitdiff
path: root/t/t5303-pack-corruption-resilience.sh
AgeCommit message (Collapse)AuthorFilesLines
2018-08-30t5303: use printf to generate delta basesLibravatar Jeff King1-10/+10
The exact byte count of the delta base file is important. The test-delta helper will feed it to patch_delta(), which will barf if it doesn't match the size byte given in the delta. Using "echo" may end up with unexpected line endings on some platforms (e.g,. "\r\n" instead of just "\n"). This actually wouldn't cause the test to fail (since we already expect test-delta to complain about these bogus deltas), but would mean that we're not exercising the code we think we are. Let's use printf instead (which we already trust to give us byte-perfect output when we generate the deltas). While we're here, let's tighten the 5-byte result size used in the "truncated copy parameters" test. This just needs to have enough room to attempt to parse the bogus copy command, meaning 2 is sufficient. Using 5 was arbitrary and just copied from the base size; since those no longer match, it's simply confusing. Let's use a more meaningful number. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-30patch-delta: handle truncated copy parametersLibravatar Jeff King1-1/+1
When we see a delta command instructing us to copy bytes from the base, we have to read the offset and size from the delta stream. We do this without checking whether we're at the end of the stream, meaning we may read past the end of the buffer. In practice this isn't exploitable in any interesting way because: 1. Deltas are always in packfiles, so we have at least a 20-byte trailer that we'll end up reading. 2. The worst case is that we try to perform a nonsense copy from the base object into the result, based on whatever was in the pack stream next. In most cases this will simply fail due to our bounds-checks against the base or the result. But even if you carefully constructed a pack stream for which it succeeds, it wouldn't perform any delta operation that you couldn't have simply included in a non-broken form. But obviously it's poor form to read past the end of the buffer we've been given. Unfortunately there's no easy way to do a single length check, since the number of bytes we need depends on the number of bits set in the initial command byte. So we'll just check each byte as we parse. We can hide the complexity in a macro; it's ugly, but not as ugly as writing out each individual conditional. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-30patch-delta: consistently report corruptionLibravatar Jann Horn1-0/+30
When applying a delta, if we see an opcode that cannot be fulfilled (e.g., asking to write more bytes than the destination has left), we break out of our parsing loop but don't signal an explicit error. We rely on the sanity check after the loop to see if we have leftover delta bytes or didn't fill our result buffer. This can silently ignore corruption when the delta buffer ends with a bogus command and the destination buffer is already full. Instead, let's jump into the error handler directly when we see this case. Note that the tests also cover the "bad opcode" case, which already handles this correctly. Signed-off-by: Jann Horn <jannh@google.com> Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-30patch-delta: fix oob readLibravatar Jann Horn1-1/+1
If `cmd` is in the range [0x01,0x7f] and `cmd > top-data`, the `memcpy(out, data, cmd)` can copy out-of-bounds data from after `delta_buf` into `dst_buf`. This is not an exploitable bug because triggering the bug increments the `data` pointer beyond `top`, causing the `data != top` sanity check after the loop to trigger and discard the destination buffer - which means that the result of the out-of-bounds read is never used for anything. Signed-off-by: Jann Horn <jannh@google.com> Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-30t5303: test some corrupt deltasLibravatar Jeff King1-0/+59
We don't have any tests that specifically check boundary cases in patch_delta(). It obviously gets exercised by tests which read from packfiles, but it's hard to create packfiles with bogus deltas. So let's cover some obvious boundary cases: 1. commands that overflow the result buffer a. literal content from the delta b. copies from a base 2. commands where the source isn't large enough a. literal content from a truncated delta b. copies that need more bytes than the base has 3. copy commands who parameters are truncated And indeed, we have problems with both 2a and 3. I've marked these both as expect_failure, though note that because they involve reading past the end of a buffer, they will typically only be caught when run under valgrind or ASan. There's one more test here, too, which just applies a basic delta. Since all of the other tests expect failure and we don't otherwise use "test-tool delta" in the test suite, this gives a sanity check that the tool works at all. These are based on an earlier patch by Jann Horn <jannh@google.com>. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-27t/helper: merge test-genrandom into test-toolLibravatar Nguyễn Thái Ngọc Duy1-5/+5
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-28t/t5303-pack-corruption-resilience.sh: use the $( ... ) construct for ↵Libravatar Elia Pinto1-8/+8
command substitution The Git CodingGuidelines prefer the $(...) construct for command substitution instead of using the backquotes `...`. The backquoted form is the traditional method for command substitution, and is supported by POSIX. However, all but the simplest uses become complicated quickly. In particular, embedded command substitutions and/or the use of double quotes require careful escaping with the backslash character. The patch was generated by: for _f in $(find . -name "*.sh") do perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg' "${_f}" done and then carefully proof-read. Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-10-29t: use perl instead of "$PERL_PATH" where applicableLibravatar Jeff King1-2/+2
As of the last commit, we can use "perl" instead of "$PERL_PATH" when running tests, as the former is now a function which uses the latter. As the shorter "perl" is easier on the eyes, let's switch to using it everywhere. This is not quite a mechanical s/$PERL_PATH/perl/ replacement, though. There are some places where we invoke perl from a script we generate on the fly, and those scripts do not have access to our internal shell functions. The result can be double-checked by running: ln -s /bin/false bin-wrappers/perl make test which continues to pass even after this patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-06-14unpack_entry: do not die when we fail to apply a deltaLibravatar Jeff King1-0/+27
When we try to load an object from disk and fail, our general strategy is to see if we can get it from somewhere else (e.g., a loose object). That lets users fix corruption problems by copying known-good versions of objects into the object database. We already handle the case where we were not able to read the delta from disk. However, when we find that the delta we read does not apply, we simply die. This case is harder to trigger, as corruption in the delta data itself would trigger a crc error from zlib. However, a corruption that pointed us at the wrong delta base might cause it. We can do the same "fail and try to find the object elsewhere" trick instead of dying. This not only gives us a chance to recover, but also puts us on code paths that will alert the user to the problem (with the current message, they do not even know which sha1 caused the problem). Note that unlike some other pack corruptions, we do not recover automatically from this case when doing a repack. There is nothing apparently wrong with the delta, as it points to a valid, accessible object, and we realize the error only when the resulting size does not match up. And in theory, one could even have a case where the corrupted size is the same, and the problem would only be noticed by recomputing the sha1. We can get around this by recomputing the deltas with --no-reuse-delta, which our test does (and this is probably good advice for anyone recovering from pack corruption). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-06-14t5303: drop "count=1" from corruption ddLibravatar Jeff King1-1/+1
This test corrupts pack objects by using "dd" with a seek command. It passes "count=1 bs=1" to munge just a single byte. However, the test added in commit b3118bdc wants to munge two bytes, and the second byte of corruption is silently ignored. This turned out not to impact the test, however. The idea was to reduce the "size of this entry" part of the header so that zlib runs out of input bytes while inflating the entry. That header is two bytes long, and the test reduced the value of both bytes; since we experience the problem if we are off by even 1 byte, it is sufficient to munge only the first one. Even though the test would have worked with only a single byte munged, and we could simply tweak the test to use a single byte, it makes sense to lift this 1-byte restriction from do_corrupt_object. It will allow future tests that do need to change multiple bytes to do so. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-24tests: enclose $PERL_PATH in double quotesLibravatar Junio C Hamano1-2/+2
Otherwise it will be split at a space after "Program" when it is set to "\\Program Files\perl" or something silly like that. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-12t: Replace 'perl' by $PERL_PATHLibravatar Vincent van Ravesteijn1-2/+2
GIT-BUILD-OPTIONS defines PERL_PATH to be used in the test suite. Only a few tests already actually use this variable when perl is needed. The other test just call 'perl' and it might happen that the wrong perl interpreter is used. This becomes problematic on Windows, when the perl interpreter that is compiled and installed on the Windows system is used, because this perl interpreter might introduce some unexpected LF->CRLF conversions. This patch makes sure that $PERL_PATH is used everywhere in the test suite and that the correct perl interpreter is used. Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-14sha1_file: Fix infinite loop when pack is corruptedLibravatar Shawn O. Pearce1-0/+9
Some types of corruption to a pack may confuse the deflate stream which stores an object. In Andy's reported case a 36 byte region of the pack was overwritten, leading to what appeared to be a valid deflate stream that was trying to produce a result larger than our allocated output buffer could accept. Z_BUF_ERROR is returned from inflate() if either the input buffer needs more input bytes, or the output buffer has run out of space. Previously we only considered the former case, as it meant we needed to move the stream's input buffer to the next window in the pack. We now abort the loop if inflate() returns Z_BUF_ERROR without consuming the entire input buffer it was given, or has filled the entire output buffer but has not yet returned Z_STREAM_END. Either state is a clear indicator that this loop is not working as expected, and should not continue. This problem cannot occur with loose objects as we open the entire loose object as a single buffer and treat Z_BUF_ERROR as an error. Reported-by: Andy Isaacson <adi@hexapodia.org> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Acked-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-03-19t5300, t5302, t5303: Do not use /dev/zeroLibravatar Johannes Sixt1-5/+7
We do not have /dev/zero on Windows. This replaces it by data generated with printf, perl, or echo. Most of the cases do not depend on that the data is a stream of zero bytes, so we use something printable; nor is an unlimited stream of data needed, so we produce only as many bytes as the test cases need. Signed-off-by: Johannes Sixt <j6t@kdbg.org>
2008-11-09t5303: fix printf format string for portabilityLibravatar Junio C Hamano1-1/+1
printf "\x01" is bad; write printf "\001" for portability. Testing with dash is a good way to find this kind of POSIX.1 violation breakages. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-11-09t5303: work around printf breakage in dashLibravatar Junio C Hamano1-1/+1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-11-02extend test coverage for latest pack corruption resilience improvementsLibravatar Nicolas Pitre1-7/+89
Signed-off-by: Nicolas Pitre <nico@cam.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-06-25Fix use of "perl -i" on WindowsLibravatar Alex Riesen1-2/+2
The perldiag(1) has following to say about this: "Can't do inplace edit without backup" (F) You're on a system such as MS-DOS that gets confused if you try reading from a deleted (but still opened) file. You have to say -i.bak, or some such. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-06-23test case for pack resilience against corruptionsLibravatar Nicolas Pitre1-0/+194
Signed-off-by: Nicolas Pitre <nico@cam.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>