OSS-Fuzz Fixes #1008
OSS-Fuzz Fixes #1008
Conversation
`strlen(buf)` is not guaranteed to return the size of the buffer, as it might contain some null bytes in the middle. Fixes htacg#1001
The loop was introduced 91f29ea when switching to a non-recursive algorithm. `InlineDup` should not be called when `ParsePre` restarts. This should fix the oss-fuzz build failure @ https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=36721
NormalizeSpaces decodes and re-encodes UTF-8 characters while looking to replace non-breaking spaces with regular spaces. When the UTF-8 decoding hits an error, a replacement character (0xFFFD) is returned and re-encoded as a 3-byte UTF-8 character. In some cases, this increases the size of strings, leading to writing past the end of the allocated buffer. Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=13191.
|
Closing, then re-opening. Hopefully this triggers the CI, which should have run. |
|
@alpire, thanks for the PR, and sorry for taking so long to get to it. Things are a bit behind since @geoffmcl has been absent. I'm especially sorry that the integration testing didn't run automatically when you posted the PR; you wouldn't have wasted time waiting for me to force it to run manually. As you can see, though, there seem to be regression test failures on two out of the three Windows builds. Given that Cygwin passed and the other two environments are Unix-like, too, it's probably a Windows-specific bug. I'd be happy to merge this PR if you can correct the issue. At some point, I may have time to work on it myself, but as you can see, there's a HUGE backlog. Thanks! |
The mmaped IO implementation on windows does not always increment `pos` on each `getByte`. It relies on an incrementing pointer `iter`, and update `pos` only when a new chunk gets maped. However, mmaped_eof was only considering `pos` and not `iter`, and therefore was sometimes incorrectly returning false until a later call to `getByte` would update `pos`.
|
@balthisar Thanks for taking a look at the PR. The failing test was the one I just added, but the failure was (surprisingly) unrelated to the PR changes. The bug was in the windows mapped IO EOF handling. A fix is available in 5f3eb66, which hopefully won't break other tests on windows. |
This MR fixes a number of issues reported by OSS-Fuzz.
Infinite loop in
ParsePreThe loop was introduced 91f29ea when switching to a non-recursive algorithm.
InlineDupshould not be called whenParsePrerestarts. This issue has been preventing OSS-Fuzz builds for a few months, which means recent code changes haven't been fuzzed yet.OSS-Fuzz Issue
Fix initialization of stack buffer in
strrepstrlen(buf)is not guaranteed to return the size of the buffer, as it might contain some null bytes in the middle, so this PR switches tosizeofinstead. Fixes #1001. This wasn't reported by OSS-Fuzz due to the build failure above, but it triggers quickly when running the fuzzer locally.Fix out-of-bounds write in
NormalizeSpacesNormalizeSpacesdecodes and re-encodes UTF-8 characters while looking to replace non-breaking spaces with regular spaces. When the UTF-8 decoding hits an error, a replacement character (0xFFFD) is returned and re-encoded as a 3-byte UTF-8 character. In some cases, this increases the size of strings, leading to writing past the end of the allocated buffer.Note that this changes the behavior of the function in the case of invalid UTF-8 strings. This is my first time contributing to the project, so it was difficult to determine if the behavior change made sense even though the tests are passing. I'd be happy to try another fix if you had another solution in mind.
OSS-Fuzz Issue