-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
assertions triggered #59
Comments
Hi Aaron, Thank you for putting this in. I could not replicate the problem. I tried The exception is occurring in the significance propagation pass where a coefficient has to be zero before it is changed to +1 or -1. Hope this helps. Kind regards, |
Hi Aous, Thanks for looking at this. It looks like the problem is that when I parse the bit stream, I get two passes for this code block, but Is this a reasonable condition to add to the block decoder ?
Thanks, |
Hi Aaron, Thank you for this suggestion. I think it is very reasonable. Kind regards, |
Thanks, Aous. I have another exception with another code block from the same file I linked to.
|
Hi Aaron, Thank you for this test; it is very important.
in two places in the decoder. To fix the current issue I suggest you move the second of these two lines from 1437-1438 to I like the way you sent me the offending codeblock. It was easy for me to added to my code and test it. I only needed codeblock width and height, so I used the default 64x64. Thank you for all the help. Kind regards, |
Sorry line numbers. I am preparing the commit. Edit: I need to think about it more. I will commit it tomorrow. |
Hi Aous,. |
Aous, I found another issue :
|
…r than missing_msbs+1. There is also a hardening step to prevent more than 1 coding pass if the length of subsquent passes is 0. Also we do not support more than 3 coding pass -- it is possible to have more than 3 coding passes; in this case, the decoder should only decode the last cleanup pass, and subsequent significance propagation pass and magnitude refinement pass, if they exist. We do not support this.
Hi Aaron, Thank you for this issue. It is useful to give some background. This block and the previous block have this issue. I am not very sure if I should flag such cases. For a reference implementation, you would be interested in this, but for a decoder, I think it is reasonable to ignore these case to make decoding faster, if they do not case a fault, a buffer overrun, or the like. The assert occurred because we have 0xFFFF in the sequence, and the assert tests for sequences larger than 0xFF8F. I am fine with decoding a malformed code block to generate erroneous decoded data. Thank you. Kind regards, PS. I just made a commit addressing the previous issues in this issue. |
Hi Aous, Thanks a lot for taking a look. So, yes, bit stuffing is expected by the decoder. It would be interesting to benchmark with and without extra checks for markers. My guess is that we wouldn't see a noticeable difference, at least on the CPU. If the checks are not there, fuzzers will easily create images that will crash the decoder, and possibly cause security issues. Also, thank you for fixing the previous issue. Best, |
Hi Aaron, Thank you for the feedback. I agree with you that detecting bitstuffing errors might not be noticeable. I have just commented out that assert. See how it goes with you. Thank you for your continuous support. Kind regards, |
Thanks, Aous. I have commented out all asserts in my code, and there are no valgrind errors. So, it looks like it's safe to do this. |
Hi Aous,
it seems like it's still possible to find unexpected conditions in the block coder.
0658dfcd793f1879b56d8959be7a175e85fe8e70.zip
Above is a truncated file, but it does trigger the exception:
ojph_decode_codeblock : line 1789 assert(dp[stride] == 0);
Thanks!
Aaron
The text was updated successfully, but these errors were encountered: