-
Notifications
You must be signed in to change notification settings - Fork 278
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
Bound recursion depth to avoid stack exhaustion #1091
Bound recursion depth to avoid stack exhaustion #1091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
You are right about you last statement. At the moment we cannot specify that some tests should only run in a specific build mode. It would be great if we could implement such feature.
// Fix for https://github.com/Exiv2/exiv2/issues/712 | ||
// A malicious file can cause a very deep recursion, leading to | ||
// stack exhaustion. | ||
if (depth > 200) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 200
an arbitrary value or is somehow a known limit ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, 200 is an arbitrary value. The value of depth
determines the amount of indentation inserted by the pretty-printer. So the output becomes unreadable as soon as depth
exceeds 80 characters or so. That's why 200 seemed like a reasonable cut-off to me. I'll add this as a comment in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is arbitrary, however it's not only the pretty printer that's in trouble. If you're 200 levels of recursion here, you've hit an undetected circular reference in the TIFF file. Typically depth < 4!
I have an open issue #547 about the lack of circular reference detection it printStructure()
for tiffs.. I'll do some work on 0.27.3 about this.
Codecov Report
@@ Coverage Diff @@
## master #1091 +/- ##
==========================================
+ Coverage 71.81% 71.83% +0.02%
==========================================
Files 152 152
Lines 17651 17654 +3
==========================================
+ Hits 12676 12682 +6
+ Misses 4975 4972 -3
Continue to review full report at Codecov.
|
Pull request has been modified.
Fix submitted into 0.27-maintenance #1140 |
Fixes #712 .
Note: I added a test, but I omitted the
-pR
flag from the command line because that feature is only supported in debug builds. As far as I know, it isn't possible to specify that the test should only run on debug builds.