Closed
Bug 1405717
Opened 7 years ago
Closed 7 years ago
Check if wrapWithProto can be made fuzzing-safe
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: decoder, Assigned: jandem)
References
(Blocks 1 open bug)
Details
(Keywords: sec-want, Whiteboard: [js:tech-debt][js:testability][adv-main58-])
Attachments
(1 file)
3.50 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
Apparently the testing function wrapWithProto is capable of covering some code in js/src/proxy/Proxy.cpp that we cover with tests but not in fuzzing. The code in question is this: http://searchfox.org/mozilla-central/rev/e62604a97286f49993eb29c493672c17c7398da9/js/src/proxy/Proxy.cpp#340 We have e.g. js/src/jit-test/tests/basic/bug842940.js that uses this and hits the mentioned code location. It seems that this wasn't always fuzzing-unsafe because bug 842940 itself is a fuzzing bug. NI for jandem based on IRC conversation.
Flags: needinfo?(jdemooij)
Updated•7 years ago
|
Priority: -- → P3
Whiteboard: [js:tech-debt][js:testability]
Assignee | ||
Comment 1•7 years ago
|
||
So bug 1126105 made this fuzzing-unsafe, and added this comment: " Note: This is not fuzzing safe because it can be used to construct\n" " deeply nested wrapper chains that cannot exist in the wild."), Apparently isCallable et al could run out of stack-space. IMO making functions fuzzing-unsafe is a pretty big (and unfortunate) hammer - we can probably just check for this.
Assignee | ||
Comment 2•7 years ago
|
||
This makes wrapWithProto fuzzing-safe again. It throws an exception when trying to wrap a wrapper to avoid the overrecursion issue. No tests rely on wrapping a wrapper.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8921423 -
Flags: review?(jwalden+bmo)
Updated•7 years ago
|
Attachment #8921423 -
Flags: review?(jwalden+bmo) → review+
Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ad173cf46012 Make wrapWithProto fuzzing-safe. r=jwalden
Comment 4•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ad173cf46012
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•6 years ago
|
Whiteboard: [js:tech-debt][js:testability] → [js:tech-debt][js:testability][adv-main58-]
You need to log in
before you can comment on or make changes to this bug.
Description
•