Closed Bug 1322400 Opened 3 years ago Closed 3 years ago

Add content-exposed GC and CC functions in fuzzing builds

Categories

(Core :: XPConnect, defect)

defect
Not set

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.
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.
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.
What name do you want for the pref?
Flags: needinfo?(choller)
Depends on: 1341809
Oh, it looks like the name is "fuzzing.enabled".
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
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.
Flags: needinfo?(choller)
Jesse, does this look reasonable?
Flags: needinfo?(jschwartzentruber)
(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)
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 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 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+
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.
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
https://hg.mozilla.org/mozilla-central/rev/80a323cabf56
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Blocks: 1346339
(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.
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)
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.