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. Also, directly jump into the error handler instead of just breaking out of the loop - otherwise, data corruption would be silently ignored if the delta buffer ends with a command and the destination buffer is already full. Signed-off-by: Jann Horn <[email protected]> --- patch-delta.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/patch-delta.c b/patch-delta.c index 56e0a5ede..283fb4b75 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) - break; + if (cmd > size || cmd > top - data) + 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); -- 2.19.0.rc0.228.g281dcd1b4d0-goog
