Messages in this thread Patch in this message | | | From | Nick Terrell <> | Subject | Re: [PATCH v3] lz4: fix LZ4_decompress_safe_partial read out of bound | Date | Fri, 19 Nov 2021 18:23:24 +0000 |
| |
> On Nov 11, 2021, at 2:50 AM, Guo Xuenan <guoxuenan@huawei.com> wrote: > > When partialDecoding, it is EOF if we've either, filled the output > buffer or can't proceed with reading an offset for following match. > > In some extreme corner cases when compressed data is crusted corrupted, > UAF will occur. As reported by KASAN [1], LZ4_decompress_safe_partial > may lead to read out of bound problem during decoding. lz4 upstream has > fixed it [2] and this issue has been disscussed here [3] before. > > current decompression routine was ported from lz4 v1.8.3, bumping lib/lz4 > to v1.9.+ is certainly a huge work to be done later, so, we'd better fix > it first. > > [1] https://lore.kernel.org/all/000000000000830d1205cf7f0477@google.com/ > [2] https://github.com/lz4/lz4/commit/c5d6f8a8be3927c0bec91bcc58667a6cfad244ad# > [3] https://lore.kernel.org/all/CC666AE8-4CA4-4951-B6FB-A2EFDE3AC03B@fb.com/ > > Reported-by: syzbot+63d688f1d899c588fb71@syzkaller.appspotmail.com > Cc: hsiangkao@linux.alibaba.com > Cc: terrelln@fb.com > Cc: cyan@fb.com > Cc: cy.fan@huawei.com > Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
Sorry I’m a bit late to the party, but this looks good to me!
Reviewed-by: Nick Terrell <terrelln@fb.com>
> --- > lib/lz4/lz4_decompress.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c > index 926f4823d5ea..fd1728d94bab 100644 > --- a/lib/lz4/lz4_decompress.c > +++ b/lib/lz4/lz4_decompress.c > @@ -271,8 +271,12 @@ static FORCE_INLINE int LZ4_decompress_generic( > ip += length; > op += length; > > - /* Necessarily EOF, due to parsing restrictions */ > - if (!partialDecoding || (cpy == oend)) > + /* Necessarily EOF when !partialDecoding. > + * When partialDecoding, it is EOF if we've either > + * filled the output buffer or > + * can't proceed with reading an offset for following match. > + */ > + if (!partialDecoding || (cpy == oend) || (ip >= (iend - 2))) > break; > } else { > /* may overwrite up to WILDCOPYLENGTH beyond cpy */ > -- > 2.31.1 >
| |