apply limit conditions for all splitting strategies

instead of just for blind split.

This is in anticipation of adversarial input,
that would intentionally target the sampling pattern of the split detector.

Note that, even without this protection, splitting can never expand beyond ZSTD_COMPRESSBOUND(),
because this upper limit uses a 1KB block size worst case scenario,
and splitting never creates blocks thath small.

The protection is more to ensure that data is not expanded by more than 3-bytes per 128 KB full block,
which is a much stricter limit.
This commit is contained in:
Yann Collet
2024-10-24 11:36:56 -07:00
parent bbda1acf85
commit 90095f056d
+21 -17
View File
@@ -4499,6 +4499,11 @@ static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t src
*/ */
if (srcSize < 128 KB || blockSizeMax < 128 KB) if (srcSize < 128 KB || blockSizeMax < 128 KB)
return MIN(srcSize, blockSizeMax); return MIN(srcSize, blockSizeMax);
/* do not split incompressible data though:
* ensure a 3 bytes per full block overhead limit.
* Note: as a consequence, the first full block skips the splitting detector.
*/
if (savings < 3) return 128 KB;
/* dynamic splitting has a cpu cost for analysis, /* dynamic splitting has a cpu cost for analysis,
* due to that cost it's only used for higher levels */ * due to that cost it's only used for higher levels */
if (strat >= ZSTD_btopt) if (strat >= ZSTD_btopt)
@@ -4508,9 +4513,8 @@ static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t src
/* blind split strategy /* blind split strategy
* heuristic value, tested as being "generally better". * heuristic value, tested as being "generally better".
* no cpu cost, but can over-split homegeneous data. * no cpu cost, but can over-split homegeneous data.
* do not split incompressible data though: respect the 3 bytes per block overhead limit.
*/ */
return (savings > 3) ? 92 KB : 128 KB; return 92 KB;
} }
/*! ZSTD_compress_frameChunk() : /*! ZSTD_compress_frameChunk() :
@@ -4587,21 +4591,21 @@ static size_t ZSTD_compress_frameChunk(ZSTD_CCtx* cctx,
} }
} /* if (ZSTD_useTargetCBlockSize(&cctx->appliedParams))*/ } /* if (ZSTD_useTargetCBlockSize(&cctx->appliedParams))*/
/* @savings is employed by the blind-split strategy, /* @savings is employed to ensure that splitting doesn't worsen expansion of incompressible data.
* to authorize splitting into less-than-full blocks, * Without splitting, the maximum expansion is 3 bytes per full block.
* and thus avoid oversplitting blocks in case of incompressible data: * An adversarial input could attempt to fudge the split detector,
* when @savings is not large enough, blind split is disactivated, and full block is used instead. * and make it split incompressible data, resulting in more block headers.
* If data is incompressible, it's allowed to expand it by 3-bytes per full block. * Note that, since ZSTD_COMPRESSBOUND() assumes a worst case scenario of 1KB per block,
* For large data, a full block is 128 KB. * and the splitter never creates blocks that small (current lower limit is 8 KB),
* blind-split will instead use 92 KB as block size. * there is already no risk to expand beyond ZSTD_COMPRESSBOUND() limit.
* So it expands incompressible data by 3-bytes per 92 KB block. * But if the goal is to not expand by more than 3-bytes per 128 KB full block,
* That's an over-expansion of ((128*3) - (92*3)) / 128 = 0.84 bytes per block. * then yes, it becomes possible to make the block splitter oversplit incompressible data.
* Therefore, when data doesn't shrink, we subtract a 1 byte malus from @savings. * Using @savings, we enforce an even more conservative condition,
* This is a conservative estimate, especially as we don't count the 3-bytes header when there are savings, * requiring the presence of enough savings (at least 3 bytes) to authorize splitting,
* but it doesn't matter, the goal is not accuracy, * otherwise only full blocks are used.
* the goal is to ensure the 3-bytes expansion limit per 128 KB input can never be breached */ * But being conservative is fine,
if (cSize < blockSize) savings += (blockSize - cSize); * since splitting barely compressible blocks is not fruitful anyway */
else savings--; savings += (S64)blockSize - (S64)cSize;
ip += blockSize; ip += blockSize;
assert(remaining >= blockSize); assert(remaining >= blockSize);