Skip to content

fix oss-fuzz case 55714 #3476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 8, 2023
+69 −29
Merged

fix oss-fuzz case 55714 #3476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e04706c
fix oss-fuzz case 55714
Cyan4973 Feb 7, 2023
cfec005
fix for v0.3 blindly ported to v0.2
Cyan4973 Feb 7, 2023
7eb4471
adapt v0.3 fix to v0.1
Cyan4973 Feb 7, 2023
b20e4e9
copy fix for v0.3 to v0.4
Cyan4973 Feb 7, 2023
7a1a171
port fix for v0.3 to v0.5
Cyan4973 Feb 7, 2023
67d7a65
port fix for v0.3 to v0.6
Cyan4973 Feb 7, 2023
9419747
fix legacy decoders v0.4, v0.5 and v0.6
Cyan4973 Feb 7, 2023
c5bf6b8
add requested check for legacy decoder v0.1
Cyan4973 Feb 7, 2023
c689310
rewrite legacy v0.7 bound checks to be independent of address space o…
Cyan4973 Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions lib/legacy/zstd_v01.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1720,20 +1720,26 @@ 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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we need to keep this check, because the literals are put in the output buffer, but oend isn't reduced.

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 */
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 */

Expand Down
11 changes: 9 additions & 2 deletions lib/legacy/zstd_v02.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down
14 changes: 10 additions & 4 deletions lib/legacy/zstd_v03.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 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 */

/* 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) */
Expand Down
14 changes: 10 additions & 4 deletions lib/legacy/zstd_v04.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2828,13 +2828,19 @@ 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 (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 */

Expand Down
14 changes: 10 additions & 4 deletions lib/legacy/zstd_v05.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3182,13 +3182,19 @@ 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 (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 */

Expand Down
14 changes: 10 additions & 4 deletions lib/legacy/zstd_v06.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3322,13 +3322,19 @@ 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 (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 */

Expand Down
11 changes: 7 additions & 4 deletions lib/legacy/zstd_v07.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand All @@ -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;
Expand Down