zstd: fixed possible 'rtbTable' underflow in FSE_normalizeCount()
Subsystems (2)
PATCH v3 1/1 zstd: fixed possible 'rtbTable' underflow in FSE_normalizeCount()
'rtbTable' may be underflowed because 'proba' is used without checking for a non-negative as index of rtbTable[].
Add check: proba >= 0
--- lib/zstd/compress/fse_compress.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
lib/zstd/compress/fse_compress.c +4 -3
@@ -492,9 +492,10 @@ size_t FSE_normalizeCount (short* normalizedCounter, unsigned tableLog, stillToDistribute--; }else {short proba = (short )((count[s]*step) >> scale); -if (proba<8 ) { -U64 restToBeat = vStep * rtbTable[proba]; - proba += (count[s]*step) - ((U64 )proba<<scale) > restToBeat; +if ((proba >=0 ) && (proba <8 )) { +U64 restToBeat = vStep * rtbTable[proba]; + + proba += (count[s]*step) - ((U64 )proba<<scale) > restToBeat; }if (proba > largestP) { largestP=proba; largest=s; } normalizedCounter[s] = proba;
Signature · 1 line
2.43.0
Ilya, can you share any context for this patch? Do you have any evidence that proba can be negative?
A discussion was just started about this patch on the zstd repo [0]. I'm happy to discuss this here or there, whichever is more convenient.
But to my first pass inspection, this seems to be protecting an impossible situation. (Separately: if it could happen, the correct behavior would to catch it and return an error, not just skip it like this patch proposes.)
Thanks,
Felix
[0] https://github.com/facebook/zstd/issues/4567
Quoted 30 lines · Ilya Krutskih
'rtbTable' may be underflowed because 'proba' is used without checking for a non-negative as index of rtbTable[].
Add check: proba >= 0
Cc [email protected] # v5.10+Fixes e0c1b49f5b67 ("lib: zstd: Upgrade to latest upstream zstd version 1.4.10")Signed-off-by Ilya Krutskih <[email protected]>--- lib/zstd/compress/fse_compress.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)lib/zstd/compress/fse_compress.c +4
-3@@ -492,9 +492,10 @@ size_t FSE_normalizeCount (short* normalizedCounter, unsigned tableLog, stillToDistribute--; }else {short proba = (short )((count[s]*step) >> scale); -if (proba<8 ) { -U64 restToBeat = vStep * rtbTable[proba]; - proba += (count[s]*step) - ((U64 )proba<<scale) > restToBeat; +if ((proba >=0 ) && (proba <8 )) { +U64 restToBeat = vStep * rtbTable[proba]; + + proba += (count[s]*step) - ((U64 )proba<<scale) > restToBeat; }if (proba > largestP) { largestP=proba; largest=s; } normalizedCounter[s] = proba;