Bug 46410 - Stack overflow when converting an Error object to string
Summary: Stack overflow when converting an Error object to string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks: 42959
  Show dependency treegraph
 
Reported: 2010-09-23 14:58 PDT by Alexey Proskuryakov
Modified: 2011-01-19 17:54 PST (History)
4 users (show)

See Also:


Attachments
test case (will crash) (340 bytes, text/html)
2010-09-23 14:58 PDT, Alexey Proskuryakov
no flags Details
Patch (21.47 KB, patch)
2011-01-07 23:03 PST, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (24.19 KB, patch)
2011-01-18 18:31 PST, Darin Adler
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-09-23 14:58:34 PDT
Created attachment 68598 [details]
test case (will crash)

If an Error object has itself as one of its properties, we crash in toString.
Comment 1 Geoffrey Garen 2010-09-23 15:51:45 PDT
<rdar://problem/8471921>
Comment 2 Eric Seidel (no email) 2010-09-29 13:56:45 PDT
It seems like this may be a generic problem with native methods, particularly any toString implementation which displays contents.

For example: do we handle the case where an Array has itself as a member?  Or do we check for array cycles during insertion?
Comment 3 Geoffrey Garen 2010-09-29 14:28:45 PDT
> For example: do we handle the case where an Array has itself as a member?

Yes.

>  Or do we check for array cycles during insertion?

No.
Comment 4 Eric Seidel (no email) 2010-09-29 14:36:53 PDT
I should also note: I do not believe that this stack overflow need be marked as a security bug.  But it's possible I don't understand the full consequences of a stack overflow bug.
Comment 5 Alexey Proskuryakov 2010-09-29 16:07:46 PDT
Yes, this bug is not marked as a security one.
Comment 6 Eric Seidel (no email) 2010-09-29 16:43:05 PDT
My mistake.
Comment 7 Darin Adler 2011-01-07 23:03:19 PST
Created attachment 78305 [details]
Patch
Comment 8 Darin Adler 2011-01-18 18:31:29 PST
Created attachment 79377 [details]
Patch
Comment 9 Geoffrey Garen 2011-01-18 18:37:58 PST
Comment on attachment 79377 [details]
Patch

Probably even better to deploy StackBounds::recursionCheck instead of continuing to use our legacy system of fixed recursion constants. But I will not make the perfect the enemy of the good!

r=me
Comment 10 Darin Adler 2011-01-19 17:54:11 PST
Committed r76185: <http://trac.webkit.org/changeset/76185>