Make MAX_IFD_NESTING_LEVEL an actual nesting level #5976
Conversation
|
I'm seeing OOMs during fuzzing, so we might still need a global (but much higher) limit or something. |
nikic
force-pushed
the
exif-nesting-level
branch
from
977c5a7 to
a5b6941
Compare
|
Okay, I have now split this into two limits: The nesting limit, and the total limit on the number of tags. The previous "nesting limit" was effectively the "total limit". I've set the nesting limit to a low number (10) and the total limit to a high number (1000). |
|
Fuzzing has been running for a few hours now and hasn't found any further issues. |
|
This would fix https://bugs.php.net/bug.php?id=80016 as well. I think we should ship this ASAP. Or do you see issues with this PR, @KalleZ? |
There was a problem hiding this comment.
I don’t see any issues with the patch, in fact it makes it makes it easier to also adjust the limits should they become an issue in the future without letting one run wild, as I had to increase this limit a few times over the years to compensate for new formats added
|
Merged as 376bbbd. |
Currently we only ever increment
ifd_nesting_level, so this ends up being a limit on the total number of IFD tags and we regularly get bug reports of it being exceeded. I think the intention behind this limit was to prevent recursion stack overflow, and for that we only need to check actual recursive usage. I've implemented that here, and dropped the nesting limit down to a smaller value (which still passes our tests).This should fix a number of bugs:
https://bugs.php.net/bug.php?id=78083
https://bugs.php.net/bug.php?id=78701
https://bugs.php.net/bug.php?id=79907