Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix NCountWriteBound #2779

Merged
merged 2 commits into from
Sep 22, 2021
Merged

Fix NCountWriteBound #2779

merged 2 commits into from
Sep 22, 2021

Conversation

senhuang42
Copy link
Contributor

OSS-Fuzz discovered an error in FSE_writeNCount().

Problem distribution:

(tableLog == 5, maxSymbolValue == 10)
[1,0,1,0,1,0,1,0,1,9,18]

Write: 6, 6, 5, 5, 5, 5, 5, 5, 5, 5, 1 bits
Total: 53 bits

But, main loop prior to flush currently writes 8 bytes rather than 7. Why?
BitCount initialized at 4, and first two symbols require an additional bit each, so we actually need 4 + 2 + 53 = 59 bits.

Currently calculated bound = 55 bits ((10+1)*5). So we need to add 4 + 2 to this bound.

@Cyan4973
Copy link
Contributor

Cyan4973 commented Sep 15, 2021

Fix looks good !

Does that seem possible to add one test case that triggers this situation (and fails without this PR) ?

Copy link
Contributor

@terrelln terrelln left a comment

Choose a reason for hiding this comment

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

Looks good once a test case is added!

@senhuang42
Copy link
Contributor Author

Test case added (fails when fuzzer.c is built with ASAN)

@senhuang42 senhuang42 merged commit 044c8b4 into facebook:dev Sep 22, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants