patch-delta: fix oob read

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>

diff --git a/patch-delta.c b/patch-delta.c
index 56e0a5ede22c9396fc897bf1d3444dce92d8916f..b937afd2c99c8ac40b2eaa37620a6942e46f8b4c 100644 (file)
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -56,7 +56,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
                        out += cp_size;
                        size -= cp_size;
                } else if (cmd) {
-                       if (cmd > size)
+                       if (cmd > size || cmd > top - data)
                                break;
                        memcpy(out, data, cmd);
                        out += cmd;

diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 912e659acf60a034ab72448b8d5b9fb4ae90f04c..7114c31ade75e6ef551c273cafa83fcfc0ba7c00 100755 (executable)
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -341,7 +341,7 @@ test_expect_success \
 # \0 - empty base
 # \2 - two bytes in result
 # \2 - two literal bytes (we are short one)
-test_expect_failure \
+test_expect_success \
     'apply delta with too few literal bytes' \
     'printf "\0\2\2X" > truncated_delta &&
      test_must_fail test-tool delta -p /dev/null truncated_delta /dev/null'