From fa72f90e7a5cfbbc32860c6336628c96791b5af3 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Thu, 30 Aug 2018 03:10:26 -0400 Subject: 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 Signed-off-by: Jeff King Reviewed-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- patch-delta.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'patch-delta.c') diff --git a/patch-delta.c b/patch-delta.c index b937afd2c9..283fb4b759 100644 --- a/patch-delta.c +++ b/patch-delta.c @@ -51,13 +51,13 @@ void *patch_delta(const void *src_buf, unsigned long src_size, if (unsigned_add_overflows(cp_off, cp_size) || cp_off + cp_size > src_size || cp_size > size) - break; + goto bad_length; memcpy(out, (char *) src_buf + cp_off, cp_size); out += cp_size; size -= cp_size; } else if (cmd) { if (cmd > size || cmd > top - data) - break; + goto bad_length; memcpy(out, data, cmd); out += cmd; data += cmd; @@ -75,6 +75,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size, /* sanity check */ if (data != top || size != 0) { + bad_length: error("delta replay has gone wild"); bad: free(dst_buf); -- cgit v1.2.3