@matheusbsbs / matheus-bagliano / commits / fa72f90e7a

patch-delta: consistently report corruption

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>

Jann Horn committed Aug 30, 2018 at 03:10 UTC fa72f90e7a5cfbbc32860c6336628c96791b5af3

2 files changed +33 -2

patch-delta.c

+3 -2

@@ -51,13 +51,13 @@ void *patch_delta(const void *src_buf, unsigned long src_size,

51 if (unsigned_add_overflows(cp_off, cp_size) ||

52 cp_off + cp_size > src_size ||

53 cp_size > size)

54 - break;

54 + goto bad_length;

55 memcpy(out, (char *) src_buf + cp_off, cp_size);

56 out += cp_size;

57 size -= cp_size;

58 } else if (cmd) {

59 if (cmd > size || cmd > top - data)

60 - break;

60 + goto bad_length;

61 memcpy(out, data, cmd);

62 out += cmd;

63 data += cmd;

@@ -75,6 +75,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,

76 /* sanity check */

77 if (data != top || size != 0) {

78 + bad_length:

79 error("delta replay has gone wild");

80 bad:

81 free(dst_buf);

t/t5303-pack-corruption-resilience.sh

+30

@@ -370,4 +370,34 @@ test_expect_failure \

370 echo base >base &&

371 test_must_fail test-tool delta -p base truncated_copy_delta /dev/null'

372

373 +# \0 - empty base

374 +# \1 - one byte in result

375 +# \1 - one literal byte (X)

376 +# \1 - trailing garbage command

377 +test_expect_success \

378 + 'apply delta with trailing garbage literal' \

379 + 'printf "\0\1\1X\1" > tail_garbage_literal &&

380 + test_must_fail test-tool delta -p /dev/null tail_garbage_literal /dev/null'

381 +

382 +# \5 - five bytes in base

383 +# \1 - one byte in result

384 +# \1 - one literal byte (X)

385 +# \221 - copy, one byte offset, one byte size

386 +# \0 - copy from offset 0

387 +# \1 - copy 1 byte

388 +test_expect_success \

389 + 'apply delta with trailing garbage copy' \

390 + 'printf "\5\1\1X\221\0\1" > tail_garbage_copy &&

391 + echo base >base &&

392 + test_must_fail test-tool delta -p /dev/null tail_garbage_copy /dev/null'

393 +

394 +# \0 - empty base

395 +# \1 - one byte in result

396 +# \1 - one literal byte (X)

397 +# \0 - bogus opcode

398 +test_expect_success \

399 + 'apply delta with trailing garbage opcode' \

400 + 'printf "\0\1\1X\0" > tail_garbage_opcode &&

401 + test_must_fail test-tool delta -p /dev/null tail_garbage_opcode /dev/null'

402 +

403 test_done