Skip to content
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

Infinite loop on truncated GIF #24

Closed
jtlapp opened this issue Dec 20, 2017 · 3 comments
Closed

Infinite loop on truncated GIF #24

jtlapp opened this issue Dec 20, 2017 · 3 comments

Comments

@jtlapp
Copy link

jtlapp commented Dec 20, 2017

The decoder will go into an infinite loop upon parsing a GIF that has been truncated in such a way that the end-of-block marker never occurs. For example, this GIF.

I have a working fix, will post in a PR.

jtlapp added a commit to jtlapp/omggif that referenced this issue Dec 20, 2017
@deanm
Copy link
Owner

deanm commented Dec 20, 2017

Thanks for the bugs / fixes / test case. I'll have a look, I think moving throw from string to Error seems fine, not sure if I should take the opportunity to make better errors or error classes or whatever... I'll have a look.

There might be other places that could have infinite loops in the decoder on bad input, I'll have to look, and maybe even fuzz a bit, but I will try to fix this one. Thanks!

@jtlapp
Copy link
Author

jtlapp commented Dec 20, 2017

Sure thing. I did give it a quick once-over looking for other infinite loops, but I don't think there are any.

@jtlapp
Copy link
Author

jtlapp commented Dec 20, 2017

Oh I see you going through pushes your own mods. Should I just cancel my PR then?

deanm added a commit that referenced this issue Dec 20, 2017
Reading out of bounds of the buffer will return undefined, which failed the
idenity zero check.  This now throws an exception on a value that is undefined
or less than zero.  The alternative option would be to just try to continue,
but this would probably just lead to needing more checks further down the line
(ex in the decompression routine which also has to handle block sizes).

Note: This is normally only ever going to be used with unsigned input buffers
so a value less than zero isn't possible, but it didn't seem to hurt to add the
check anyway.
@deanm deanm closed this as completed Dec 20, 2017
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

No branches or pull requests

2 participants