FiniteStateEntropy icon indicating copy to clipboard operation
FiniteStateEntropy copied to clipboard

NULL pointer dereference in BIT_reloadDStream()

Open JohanKnutzen opened this issue 7 years ago • 7 comments

Crashing line: 430: bitD->bitContainer = MEM_readLEST(bitD->ptr); in bitstream.h in the function BIT_reloadDStream() triggered by FSE_decompressU16() with the following code:

const uint8_t* casted2 = (uint8_t*)"Àâ(¢x(Kùÿÿcb¿\a";
uint16_t out[256];
size_t ret = FSE_decompressU16(out, 256, casted2, 14);

This is caused by size_t const NSize = FSE_readNCount (NCount, &maxSymbolValue, &tableLog, istart, cSrcSize); returning zero in the following block of FSE_decompressU16():

{ size_t const NSize = FSE_readNCount (NCount, &maxSymbolValue, &tableLog, istart, cSrcSize); if (FSE_isError(NSize)) return NSize; ip += NSize; cSrcSize -= NSize; } thus, resulting in a zero cSrcSize when entering FSE_decompressU16_usingDTable() which is unexpected

I don't know if this can happen in the 8 bit version.

JohanKnutzen avatar Jan 03 '19 00:01 JohanKnutzen

Is this fixed in dev? I can test this one as well if it is believed to be fixed

JohanKnutzen avatar Aug 19 '20 20:08 JohanKnutzen

There have been a number of changes in fse since 2019, but I'm not sure if one of them addresses this concern. FSE_readNCount was updated in zstd, and maybe it fixed this issue. Note though that not all updates are sync into fse repository yet : we do have difficulties to keep both repositories automatically synchronized, for complex path & namespace reasons, so it always requires some manual work to do that.

Cyan4973 avatar Sep 15 '20 19:09 Cyan4973

@Cyan4973 Sorry for the late reply. I just updated to the latest version of fse (dev branch) and it still crashes. The repro is still

const uint8_t* casted2 = (uint8_t*)"Àâ(¢x(Kùÿÿcb¿\a";
uint16_t out[256];
size_t ret = FSE_decompressU16(out, 256, casted2, 14);

I have a more complex repro that involves FSE_compressU16 if you'd like me to provide it.

JohanKnutzen avatar Mar 17 '21 01:03 JohanKnutzen

@Cyan4973 Bump!

JohanKnutzen avatar Aug 05 '21 18:08 JohanKnutzen

I'm sorry, there is currently no availability left to continue maintaining FSE as a separate library.

There is ongoing work on FSE, but as part of zstd, and these code bases are now diverging. And for example, the FSE_compressU16() variant is not used there, so is not supported.

The situation could be different in some future, but in the foreseeable future, I'm afraid I can't offer the time for it.

Cyan4973 avatar Aug 05 '21 18:08 Cyan4973

Alright, thanks @Cyan4973

If I find a fix for this, can I do a PR on this repo?

JohanKnutzen avatar Aug 05 '21 18:08 JohanKnutzen

Yes

Cyan4973 avatar Aug 05 '21 18:08 Cyan4973