Closed Bug 1452602 Opened 6 years ago Closed 6 years ago

Mark some shell functions as fuzzing-safe

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file)

      No description provided.
Attached patch PatchSplinter Review
Maybe we can do some fuzzing first in case this uncovers serious bugs.
Attachment #8966178 - Flags: feedback?(nth10sd)
Attachment #8966178 - Flags: feedback?(choller)
Comment on attachment 8966178 [details] [diff] [review]
Patch

This didn't immediately blow up jsfunfuzz et al, but I guess the following functions have to be added to it?

grayRoot()
addMarkObservers(array_of_objects)
clearMarkObservers()
getMarks()
Flags: needinfo?(jdemooij)
Attachment #8966178 - Flags: feedback?(nth10sd) → feedback+
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2)
> This didn't immediately blow up jsfunfuzz et al

Thanks for testing!

> but I guess the following functions have to be added to it?

Yeah, also dumpScopeChain(obj) - although that one probably isn't too important.
Flags: needinfo?(jdemooij)
Priority: -- → P3
Attachment #8966178 - Flags: review?(jcoppeard)
Comment on attachment 8966178 [details] [diff] [review]
Patch

Review of attachment 8966178 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/shell/js.cpp
@@ +6150,5 @@
> +        auto* observers =
> +            cx->new_<MarkBitObservers>(cx->runtime(), NonshrinkingGCObjectVector());
> +        if (!observers)
> +            return nullptr;
> +        sc->markObservers.reset(observers);

Is this change necessary?
Attachment #8966178 - Flags: review?(jcoppeard) → review+
Keywords: sec-other
Comment on attachment 8966178 [details] [diff] [review]
Patch

decoder said he likely tested this and suggested landing.
Attachment #8966178 - Flags: feedback?(choller)
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf2687e4e96eaff899122f321ef3391ba5a5a260

(In reply to Jon Coppeard (:jonco) from comment #4)
> Is this change necessary?

Hm you're right, maybe not strictly necessary, but I like immediately returning on OOM.
Group: javascript-core-security
Keywords: sec-other
https://hg.mozilla.org/mozilla-central/rev/cf2687e4e96e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.