Skip to content

Conversation

@1c3t3a
Copy link
Contributor

@1c3t3a 1c3t3a commented Oct 31, 2025

Currently, when getting the LZWStatus::NoProgress from the underlying weezl library, tiff checks that no input bytes were read and no output bytes were written. Although this is according to what the weezl library mentions in their doc-comment, it's not what the real result always looks like.

Our fuzzer found input that triggered exactly this case, where the LZW reader is able to write some output byte before hitting the NoProgress status.

This change removes the assert, as the resulting action is anyways to return an error that can be handled by the caller.

Currently, when getting the LZWStatus::NoProgress from the underlying
weezl library, tiff checks that no input bytes were read and no
output bytes were written. Although this is according to what the
weezl library mentions in their doc-comment, it's not what the real
result always looks like.

Our fuzzer found input that triggered exactly this case, where the LZW
reader is able to write some output byte before hitting the NoProgress
status.

This change removes the assert, as the resulting action is anyways to return
an error that can be handled by the caller.
@1c3t3a
Copy link
Contributor Author

1c3t3a commented Oct 31, 2025

Uff... this seems to hit a debug_assert in the underlying library: https://github.com/image-rs/weezl/blob/fa42aa42da4b611a407d1d4e0d41a72e16e3f1a7/src/decode.rs#L909-L927. What's the best way of moving this forward? Should we make the debug_assert configurable as mentioned by the doc comment? Should we run this crates fuzz-tests in release?

@197g
Copy link
Member

197g commented Oct 31, 2025

Do you have a minimal reproduction file / lzw stream data? I thought that debug assert was being fuzzed, you can definitely open it against weezl as well. It's very likely not critical as the condition is … complex and not solely used for optimization but it is curious either way.

@197g 197g merged commit bec6558 into image-rs:main Nov 4, 2025
10 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants