From: Jann Horn <jannh@google.com>
Date: Thu, 30 Aug 2018 13:25:10 GMT
Subject: Re: [PATCH 0/5] handle corruption in patch-delta
Message-ID: <CAG48ez1uK9AhmodAVH_hWuzLfVQh1=mp2=WQcUaNoEQ8nMHtpw@mail.gmail.com>
In-Reply-To: <20180830070548.GA15081@sigill.intra.peff.net>

On Thu, Aug 30, 2018 at 9:05 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Aug 29, 2018 at 10:58:55PM +0200, Jann Horn wrote:
>
> > 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.
>
> Based on my earlier observations, here's a replacement patch series I
> came up with. It has:
[...]
> I hope you don't mind me hacking up your patches a bit.

Not at all. I'm happy that I don't have to write a v2 series.

> Thanks again for your original report and patch.

Thanks for turning my patch into something decent so quickly!