Closed
Bug 1322400
Opened 7 years ago
Closed 7 years ago
Add content-exposed GC and CC functions in fuzzing builds
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Currently the fuzzers use an addon to force GCs and CCs, but once only WebExtensions are supported, this won't work any more. It seems unlikely that regular addons want to let content run the GC and CC, so it doesn't seem worthwhile to add a WebExtension interface. At the same time, we do have a pref we use for SpecialPowers, but it enables a ton of things that could potentially be dangerous for a fuzzer to run. So I think the best approach here is to add a special pref that works kind of like security.turn_off_all_security_so_that_viruses_can_take_over_this_computer, but with less powers.
Comment 1•7 years ago
|
||
We also have a build time switch now, --enable-fuzzing. With this switch, we could automatically turn on all testing functions and powers we need, without requiring these things from an addon. In bug 1332361 I'm working on exposing the JS testing functions and the AFL runtime function to content whenever we build with --enable-fuzzing. Maybe this approach is also viable for other super powers we need during fuzzing.
Comment 2•7 years ago
|
||
After some discussions with :truber, we figured that it might make sense to protect these powers both by build flag and by runtime flag (so it can only be turned on in fuzzing builds, but fuzzing builds are still (by default) safe for browsing). That way, if we decide to make our ASan build fuzzable by default, we can still use them for browsing as well.
Assignee | ||
Comment 4•7 years ago
|
||
Oh, it looks like the name is "fuzzing.enabled".
Assignee | ||
Comment 5•7 years ago
|
||
I'm just going to add GC and CC to start with. Once the basic framework is in place, adding more should be easy.
Summary: Add pref for fuzzing functions → Add content-exposed GC and CC functions in fuzzing builds
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
With this patch, in fuzzing enabled builds where fuzzing.enabled is set, the following can be used in a content web page to call the GC or CC, respectively: FuzzingFunctions.garbageCollect(); FuzzingFunctions.cycleCollect(); This only works in on the main thread, but it is a start.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(choller)
Assignee | ||
Comment 8•7 years ago
|
||
Jesse, does this look reasonable?
Flags: needinfo?(jschwartzentruber)
Comment 9•7 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #8) > Jesse, does this look reasonable? Looks great! Thanks for doing this. Just to verify: GarbageCollect looks equivalent to fuzzPriv.forceGC (which called nsXPCComponents_Utils::ForceGC) and CycleCollect looks equivalent to fuzzPriv.CC (which called nsDOMWindowUtils::CycleCollect)
Flags: needinfo?(jschwartzentruber)
Assignee | ||
Comment 10•7 years ago
|
||
Yes, that is right. I think the current functions take arguments, but in a little bit of searching on bugzilla for bugs with "fuzzPriv" in there somewhere I didn't see anybody use the arguments.
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8840200 [details] Bug 1322400 - Add content-exposed GC and CC functions to fuzzing builds. https://reviewboard.mozilla.org/r/114694/#review116844 ::: dom/base/FuzzingFunctions.h:19 (Diff revision 2) > + > +class FuzzingFunctions final > +{ > +public: > + static void > + GarbageCollect(const GlobalObject& global); aGlobal ::: dom/base/FuzzingFunctions.h:22 (Diff revision 2) > +public: > + static void > + GarbageCollect(const GlobalObject& global); > + > + static void > + CycleCollect(const GlobalObject& global); ditto ::: dom/base/FuzzingFunctions.cpp:16 (Diff revision 2) > + > +namespace mozilla { > +namespace dom { > + > +/* static */ void > +FuzzingFunctions::GarbageCollect(const GlobalObject& global) Nit, aGlobal ::: dom/base/FuzzingFunctions.cpp:24 (Diff revision 2) > + nsJSContext::NonIncrementalGC, > + nsJSContext::NonShrinkingGC); > +} > + > +/* static */ void > +FuzzingFunctions::CycleCollect(const GlobalObject& global) ditto ::: dom/webidl/moz.build:1056 (Diff revision 2) > if CONFIG['MOZ_WIDGET_TOOLKIT'] != 'gonk': > WEBIDL_FILES += [ > 'InstallTrigger.webidl', > ] > > +if CONFIG['FUZZING']: I guess this is fine, though just relying on the Pref might be ok too. If we relied on some Pref, one could set it even on normal builds. But up to you
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8840200 [details] Bug 1322400 - Add content-exposed GC and CC functions to fuzzing builds. https://reviewboard.mozilla.org/r/114694/#review116846 I guess we don't need any ifdefs in Bindings.conf
Attachment #8840200 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 14•7 years ago
|
||
Oops. I'll just delete all of the |global| variables because I don't use them.
> If we relied on some Pref, one could set it even on normal builds. But up to you
The fuzzing people asked for it like this, so that's why I'm doing it like this. These functions are fairly benign so it isn't a big deal but maybe in the future we'll add more dangerous things.
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/80a323cabf56 Add content-exposed GC and CC functions to fuzzing builds. r=smaug
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/80a323cabf56
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 18•7 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #12) > If we relied on some Pref, one could set it even on normal builds. But up to > you This kind of approach is generally considered harmful by the security team. I think the reasoning is that people can be (and have been) tricked into setting prefs. GC and CC might not be so problematic, but we have other functions that could be abused. So in order to be on the safe side here, I decided that we will protect all of the fuzzing functions with a build time switch so nobody can accidentially turn their browser unsafe.
Comment 19•7 years ago
|
||
Andrew, are these functions now also available in xpcshell? If not, can we expose them there? I tried to import FuzzingFunctions with importGlobalProperties but that didn't work. Or does CC not exist in xpcshell? I can use the JSTestingFunctions if CC is not a thing there.
Flags: needinfo?(continuation)
Assignee | ||
Comment 20•7 years ago
|
||
I don't know how things are exposed in XPCShell. I think we do have the CC there. importGlobalProperties has a whitelist, I think.
Flags: needinfo?(continuation)
You need to log in
before you can comment on or make changes to this bug.
Description
•