check patch_delta bounds more carefully

Let's avoid going south with invalid delta data.

Signed-off-by: Nicolas Pitre <nico@cam.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/delta.h b/delta.h
index a15350dabcd497d4cb21261721706eca072de034..9464f3e9b08b959d11305688ce194867a4017817 100644 (file)
--- a/delta.h
+++ b/delta.h
@@ -16,7 +16,8 @@ extern void *patch_delta(void *src_buf, unsigned long src_size,
  * This must be called twice on the delta data buffer, first to get the
  * expected reference buffer size, and again to get the result buffer size.
  */
  * This must be called twice on the delta data buffer, first to get the
  * expected reference buffer size, and again to get the result buffer size.
  */

-static inline unsigned long get_delta_hdr_size(const unsigned char **datap)
+static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
+                                              const unsigned char *top)

 {
        const unsigned char *data = *datap;
        unsigned char cmd;
 {
        const unsigned char *data = *datap;
        unsigned char cmd;


@@ -26,7 +27,7 @@ static inline unsigned long get_delta_hdr_size(const unsigned char **datap)
                cmd = *data++;
                size |= (cmd & ~0x80) << i;
                i += 7;
                cmd = *data++;
                size |= (cmd & ~0x80) << i;
                i += 7;

-       } while (cmd & 0x80);
+       } while (cmd & 0x80 && data < top);

        *datap = data;
        return size;
 }
        *datap = data;
        return size;
 }

diff --git a/patch-delta.c b/patch-delta.c
index c0e13114351c4ea427ddfe4d9e9c20a541201ad6..d95f0d9721dd87c0d4aff485e32e0a68623ee5cc 100644 (file)
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -28,12 +28,12 @@ void *patch_delta(void *src_buf, unsigned long src_size,
        top = delta_buf + delta_size;
 
        /* make sure the orig file size matches what we expect */
        top = delta_buf + delta_size;
 
        /* make sure the orig file size matches what we expect */

-       size = get_delta_hdr_size(&data);
+       size = get_delta_hdr_size(&data, top);

        if (size != src_size)
                return NULL;
 
        /* now the result size */
        if (size != src_size)
                return NULL;
 
        /* now the result size */

-       size = get_delta_hdr_size(&data);
+       size = get_delta_hdr_size(&data, top);

        dst_buf = malloc(size + 1);
        if (!dst_buf)
                return NULL;
        dst_buf = malloc(size + 1);
        if (!dst_buf)
                return NULL;


@@ -52,21 +52,37 @@ void *patch_delta(void *src_buf, unsigned long src_size,
                        if (cmd & 0x20) cp_size |= (*data++ << 8);
                        if (cmd & 0x40) cp_size |= (*data++ << 16);
                        if (cp_size == 0) cp_size = 0x10000;
                        if (cmd & 0x20) cp_size |= (*data++ << 8);
                        if (cmd & 0x40) cp_size |= (*data++ << 16);
                        if (cp_size == 0) cp_size = 0x10000;

+                       if (cp_off + cp_size < cp_size ||
+                           cp_off + cp_size > src_size ||
+                           cp_size > size)
+                               goto bad;

                        memcpy(out, src_buf + cp_off, cp_size);
                        out += cp_size;
                        memcpy(out, src_buf + cp_off, cp_size);
                        out += cp_size;

-               } else {
+                       size -= cp_size;
+               } else if (cmd) {
+                       if (cmd > size)
+                               goto bad;

                        memcpy(out, data, cmd);
                        out += cmd;
                        data += cmd;
                        memcpy(out, data, cmd);
                        out += cmd;
                        data += cmd;

+                       size -= cmd;
+               } else {
+                       /*
+                        * cmd == 0 is reserved for future encoding
+                        * extensions. In the mean time we must fail when
+                        * encountering them (might be data corruption).
+                        */
+                       goto bad;

                }
        }
 
        /* sanity check */
                }
        }
 
        /* sanity check */

-       if (data != top || out - dst_buf != size) {
+       if (data != top || size != 0) {
+               bad:

                free(dst_buf);
                return NULL;
        }
 
                free(dst_buf);
                return NULL;
        }
 

-       *dst_size = size;
+       *dst_size = out - dst_buf;

        return dst_buf;
 }
        return dst_buf;
 }

diff --git a/sha1_file.c b/sha1_file.c
index aa09b4646abff2f82d30c9968ae277d35db8c438..d8ef5655611c7271dfc51f93dc8d3859556733ba 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -804,10 +804,12 @@ static int packed_delta_info(unsigned char *base_sha1,
                 * the result size.
                 */
                data = delta_head;
                 * the result size.
                 */
                data = delta_head;

-               get_delta_hdr_size(&data); /* ignore base size */
+
+               /* ignore base size */
+               get_delta_hdr_size(&data, delta_head+sizeof(delta_head));

 
                /* Read the result size */
 
                /* Read the result size */

-               result_size = get_delta_hdr_size(&data);
+               result_size = get_delta_hdr_size(&data, delta_head+sizeof(delta_head));

                *sizep = result_size;
        }
        return 0;
                *sizep = result_size;
        }
        return 0;