From e04706c58cc462cc771292c01abc66caaebe57a6 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 6 Feb 2023 20:20:01 -0800 Subject: [PATCH 1/9] fix oss-fuzz case 55714 impacts legacy decoder v0.3 in 32-bit mode --- lib/legacy/zstd_v03.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/legacy/zstd_v03.c b/lib/legacy/zstd_v03.c index 5b1fd7175..d247d5d55 100644 --- a/lib/legacy/zstd_v03.c +++ b/lib/legacy/zstd_v03.c @@ -2706,18 +2706,24 @@ static size_t ZSTD_execSequence(BYTE* op, const BYTE* const litEnd = *litPtr + sequence.litLength; /* checks */ - if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); /* last match must start at a minimum distance of 8 from oend */ + size_t const seqLength = sequence.litLength + sequence.matchLength; + + if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); + /* Now we know there are no overflow in literal nor match lengths, can use the pointer check */ + if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); + if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); + if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ /* copy Literals */ - ZSTD_wildcopy(op, *litPtr, sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ + ZSTD_wildcopy(op, *litPtr, (ptrdiff_t)sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ op = oLitEnd; *litPtr = litEnd; /* update for next sequence */ /* copy Match */ - { - const BYTE* match = op - sequence.offset; + { const BYTE* match = op - sequence.offset; /* check */ if (sequence.offset > (size_t)op) return ERROR(corruption_detected); /* address space overflow test (this test seems kept by clang optimizer) */ From cfec005efd13b45433210ad8db543de3440fe2ad Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 6 Feb 2023 20:26:25 -0800 Subject: [PATCH 2/9] fix for v0.3 blindly ported to v0.2 in case it would be applicable here too. --- lib/legacy/zstd_v02.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/legacy/zstd_v02.c b/lib/legacy/zstd_v02.c index dfaed7bb8..e09bb4a24 100644 --- a/lib/legacy/zstd_v02.c +++ b/lib/legacy/zstd_v02.c @@ -3066,12 +3066,19 @@ static size_t ZSTD_execSequence(BYTE* op, const BYTE* const litEnd = *litPtr + sequence.litLength; /* checks */ - if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); /* last match must start at a minimum distance of 8 from oend */ + size_t const seqLength = sequence.litLength + sequence.matchLength; + + if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); + /* Now we know there are no overflow in literal nor match lengths, can use the pointer check */ + if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); + if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); + if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ /* copy Literals */ - ZSTD_wildcopy(op, *litPtr, sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ + ZSTD_wildcopy(op, *litPtr, (ptrdiff_t)sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ op = oLitEnd; *litPtr = litEnd; /* update for next sequence */ From 7eb4471fec8807b67a8475ad13cdf0310f2fad81 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 6 Feb 2023 20:33:38 -0800 Subject: [PATCH 3/9] adapt v0.3 fix to v0.1 slightly different constraints on end of buffer conditions --- lib/legacy/zstd_v01.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/legacy/zstd_v01.c b/lib/legacy/zstd_v01.c index 8c504c792..7f24cf685 100644 --- a/lib/legacy/zstd_v01.c +++ b/lib/legacy/zstd_v01.c @@ -1720,20 +1720,25 @@ static size_t ZSTD_execSequence(BYTE* op, static const int dec32table[] = {0, 1, 2, 1, 4, 4, 4, 4}; /* added */ static const int dec64table[] = {8, 8, 8, 7, 8, 9,10,11}; /* subtracted */ const BYTE* const ostart = op; + BYTE* const oLitEnd = op + sequence.litLength; const size_t litLength = sequence.litLength; BYTE* const endMatch = op + litLength + sequence.matchLength; /* risk : address space overflow (32-bits) */ const BYTE* const litEnd = *litPtr + litLength; - /* check */ + /* checks */ + size_t const seqLength = sequence.litLength + sequence.matchLength; + + if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); + /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ + if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); + if (endMatch > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ - if (litEnd > litLimit) return ERROR(corruption_detected); - if (sequence.matchLength > (size_t)(*litPtr-op)) return ERROR(dstSize_tooSmall); /* overwrite literal segment */ + if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ /* copy Literals */ - if (((size_t)(*litPtr - op) < 8) || ((size_t)(oend-litEnd) < 8) || (op+litLength > oend-8)) - memmove(op, *litPtr, litLength); /* overwrite risk */ - else - ZSTD_wildcopy(op, *litPtr, litLength); + ZSTD_memmove(op, *litPtr, sequence.litLength); /* note : v0.1 seems to allow scenarios where output or input are close to end of buffer */ + op += litLength; *litPtr = litEnd; /* update for next sequence */ From b20e4e95f2a9295adf82c4fdb13bcb1864983fd9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 6 Feb 2023 20:36:37 -0800 Subject: [PATCH 4/9] copy fix for v0.3 to v0.4 in case it would be applicable for this legacy version too. --- lib/legacy/zstd_v03.c | 2 +- lib/legacy/zstd_v04.c | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/legacy/zstd_v03.c b/lib/legacy/zstd_v03.c index d247d5d55..b0d7f521e 100644 --- a/lib/legacy/zstd_v03.c +++ b/lib/legacy/zstd_v03.c @@ -2710,7 +2710,7 @@ static size_t ZSTD_execSequence(BYTE* op, if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); - /* Now we know there are no overflow in literal nor match lengths, can use the pointer check */ + /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); diff --git a/lib/legacy/zstd_v04.c b/lib/legacy/zstd_v04.c index 23735443a..f820134b7 100644 --- a/lib/legacy/zstd_v04.c +++ b/lib/legacy/zstd_v04.c @@ -2828,13 +2828,20 @@ static size_t ZSTD_execSequence(BYTE* op, const BYTE* const litEnd = *litPtr + sequence.litLength; const BYTE* match = oLitEnd - sequence.offset; - /* check */ - if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); /* last match must start at a minimum distance of 8 from oend */ + /* checks */ + size_t const seqLength = sequence.litLength + sequence.matchLength; + + if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); + /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ + if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); + if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); + if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ - if (litEnd > litLimit) return ERROR(corruption_detected); /* risk read beyond lit buffer */ + if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ /* copy Literals */ - ZSTD_wildcopy(op, *litPtr, sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ + ZSTD_wildcopy(op, *litPtr, (ptrdiff_t)sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ op = oLitEnd; *litPtr = litEnd; /* update for next sequence */ From 7a1a1716587c7e600e7c1759daccf5c679b6d961 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 6 Feb 2023 20:40:29 -0800 Subject: [PATCH 5/9] port fix for v0.3 to v0.5 in case it would be applicable for this version too --- lib/legacy/zstd_v05.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/legacy/zstd_v05.c b/lib/legacy/zstd_v05.c index 5d2ebb2b5..85c4ce79f 100644 --- a/lib/legacy/zstd_v05.c +++ b/lib/legacy/zstd_v05.c @@ -3182,13 +3182,20 @@ static size_t ZSTDv05_execSequence(BYTE* op, const BYTE* const litEnd = *litPtr + sequence.litLength; const BYTE* match = oLitEnd - sequence.offset; - /* check */ - if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); /* last match must start at a minimum distance of 8 from oend */ + /* checks */ + size_t const seqLength = sequence.litLength + sequence.matchLength; + + if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); + /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ + if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); + if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); + if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ - if (litEnd > litLimit) return ERROR(corruption_detected); /* risk read beyond lit buffer */ + if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ /* copy Literals */ - ZSTDv05_wildcopy(op, *litPtr, sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ + ZSTDv05_wildcopy(op, *litPtr, (ptrdiff_t)sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ op = oLitEnd; *litPtr = litEnd; /* update for next sequence */ From 67d7a659f871a1bfbd99e82ad5036e619b7ca758 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 6 Feb 2023 20:43:09 -0800 Subject: [PATCH 6/9] port fix for v0.3 to v0.6 in case it would applicable for this version --- lib/legacy/zstd_v06.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/legacy/zstd_v06.c b/lib/legacy/zstd_v06.c index 53aaec468..097182a4c 100644 --- a/lib/legacy/zstd_v06.c +++ b/lib/legacy/zstd_v06.c @@ -3322,13 +3322,20 @@ static size_t ZSTDv06_execSequence(BYTE* op, const BYTE* const iLitEnd = *litPtr + sequence.litLength; const BYTE* match = oLitEnd - sequence.offset; - /* check */ - if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); /* last match must start at a minimum distance of 8 from oend */ + /* checks */ + size_t const seqLength = sequence.litLength + sequence.matchLength; + + if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); + /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ + if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); + if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); + if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ - if (iLitEnd > litLimit) return ERROR(corruption_detected); /* over-read beyond lit buffer */ + if (iLitEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ /* copy Literals */ - ZSTDv06_wildcopy(op, *litPtr, sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ + ZSTDv06_wildcopy(op, *litPtr, (ptrdiff_t)sequence.litLength); /* note : oLitEnd <= oend-8 : no risk of overwrite beyond oend */ op = oLitEnd; *litPtr = iLitEnd; /* update for next sequence */ From 94197471719757d9dcf7aca10705d829a45817c8 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 7 Feb 2023 14:02:12 -0800 Subject: [PATCH 7/9] fix legacy decoders v0.4, v0.5 and v0.6 --- lib/legacy/zstd_v04.c | 1 - lib/legacy/zstd_v05.c | 1 - lib/legacy/zstd_v06.c | 1 - 3 files changed, 3 deletions(-) diff --git a/lib/legacy/zstd_v04.c b/lib/legacy/zstd_v04.c index f820134b7..57be832bd 100644 --- a/lib/legacy/zstd_v04.c +++ b/lib/legacy/zstd_v04.c @@ -2835,7 +2835,6 @@ static size_t ZSTD_execSequence(BYTE* op, if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); - if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ diff --git a/lib/legacy/zstd_v05.c b/lib/legacy/zstd_v05.c index 85c4ce79f..93a1169f3 100644 --- a/lib/legacy/zstd_v05.c +++ b/lib/legacy/zstd_v05.c @@ -3189,7 +3189,6 @@ static size_t ZSTDv05_execSequence(BYTE* op, if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); - if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ diff --git a/lib/legacy/zstd_v06.c b/lib/legacy/zstd_v06.c index 097182a4c..175f7cc42 100644 --- a/lib/legacy/zstd_v06.c +++ b/lib/legacy/zstd_v06.c @@ -3329,7 +3329,6 @@ static size_t ZSTDv06_execSequence(BYTE* op, if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected); /* Now we know there are no overflow in literal nor match lengths, can use pointer checks */ if (oLitEnd > oend_8) return ERROR(dstSize_tooSmall); - if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected); if (oMatchEnd > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ if (iLitEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ From c5bf6b8b88456804b480176c6183a893ca01bf15 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 7 Feb 2023 14:47:16 -0800 Subject: [PATCH 8/9] add requested check for legacy decoder v0.1 which uses a different technique to store literals, and therefore must check for potential overwrites. --- lib/legacy/zstd_v01.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/legacy/zstd_v01.c b/lib/legacy/zstd_v01.c index 7f24cf685..1a3aad07e 100644 --- a/lib/legacy/zstd_v01.c +++ b/lib/legacy/zstd_v01.c @@ -1735,6 +1735,7 @@ static size_t ZSTD_execSequence(BYTE* op, if (endMatch > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */ if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */ + if (sequence.matchLength > (size_t)(*litPtr-op)) return ERROR(dstSize_tooSmall); /* overwrite literal segment */ /* copy Literals */ ZSTD_memmove(op, *litPtr, sequence.litLength); /* note : v0.1 seems to allow scenarios where output or input are close to end of buffer */ From c689310b2544c1eee6002bb4fa9a7ed8c7609dd1 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 7 Feb 2023 17:11:07 -0800 Subject: [PATCH 9/9] rewrite legacy v0.7 bound checks to be independent of address space overflow --- lib/legacy/zstd_v07.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/legacy/zstd_v07.c b/lib/legacy/zstd_v07.c index c2b88eb3e..15dc3ef79 100644 --- a/lib/legacy/zstd_v07.c +++ b/lib/legacy/zstd_v07.c @@ -3552,11 +3552,14 @@ size_t ZSTDv07_execSequence(BYTE* op, const BYTE* match = oLitEnd - sequence.offset; /* check */ - if ((oLitEnd>oend_w) | (oMatchEnd>oend)) return ERROR(dstSize_tooSmall); /* last match must start at a minimum distance of WILDCOPY_OVERLENGTH from oend */ - if (iLitEnd > litLimit) return ERROR(corruption_detected); /* over-read beyond lit buffer */ + assert(oend >= op); + if (sequence.litLength + WILDCOPY_OVERLENGTH > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + if (sequenceLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall); + assert(litLimit >= *litPtr); + if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected);; /* copy Literals */ - ZSTDv07_wildcopy(op, *litPtr, sequence.litLength); /* note : since oLitEnd <= oend-WILDCOPY_OVERLENGTH, no risk of overwrite beyond oend */ + ZSTDv07_wildcopy(op, *litPtr, (ptrdiff_t)sequence.litLength); /* note : since oLitEnd <= oend-WILDCOPY_OVERLENGTH, no risk of overwrite beyond oend */ op = oLitEnd; *litPtr = iLitEnd; /* update for next sequence */ @@ -3570,7 +3573,7 @@ size_t ZSTDv07_execSequence(BYTE* op, return sequenceLength; } /* span extDict & currentPrefixSegment */ - { size_t const length1 = dictEnd - match; + { size_t const length1 = (size_t)(dictEnd - match); memmove(oLitEnd, match, length1); op = oLitEnd + length1; sequence.matchLength -= length1;