This patch does several things. Because Workers aren't on the main thread,
many of the things done are in the name of off main thread access.
1) Changes a parameter in IsEvalAllowed from a nsIPrincipal to a bool.
We only used the principal to determined if it was the System Principal.
Principals aren't thread safe and can only be accessed on Main Thread, so
if we passed a Principal in, we would be in error. Instead only pass in
the bool which - for workers - comes from a thread-safe location.
2) Separates out the Telemetry Event Recording and sending a message to the
console into a new function nsContentSecurityUtils::NotifyEvalUsage. (And
creates a runnable that calls it.)
We do this because we will need to only call this method on the main thread.
Telemetry Event Recording has only ever been called on the Main Thread.
While I possibly-successfully cut it over to happen Off Main Thread (OMT)
by porting preferences to StaticPrefs, I don't know if there were other
threading assumptions in the Telemetry Code. So it would be much safer to
just continue recording Event Telemetry on the main thread.
Sending a message to the console requires calling GetStringBundleService()
which requires main thread. I didn't investigate if this could be made
thread-safe, I just threw it onto the main thread too.
If, in IsEvalAllowed, we are on the main thread - we call NotifyEvalUsage
directly. If we are not, we create a runnable which will then call
NotifyEvalUsage for us on the main thread.
3) Ports allow_eval_with_system_principal and allow_eval_in_parent_process
from bools to RelaxedAtomicBool - because we now check these prefs OMT.
4) In RuntimeService.cpp, adds the call to IsEvalAllowed.
5) Add resource://gre/modules/workers/require.js to the allowlist of eval
usage. This was the script that identified this gap in the first place.
It uses eval (twice) for structural reasons (scope and line number
massaging.) The contents of the eval are the result of a request to a
uri (which may be internal, like resource://). The whole point of this
is to implement a CommonJS require() api.
This usage of eval is safe because the only way an attacker can inject
into it is by either controlling the response of the uri request or
controlling (or appending to) the argument. If they can do that, they
are able to inject script into Firefox even if we cut this usage of eval
over to some other type of safe(r) script loader.
Bug 1584564 tracks making sure calls to require.js are safe.
6) Adds cld-worker.js to the allowlist. Bug 1584605 is for refactoring that
eval usage, which is decidedly non-trivial.
7) Does _not_ enforce the eval restrictions for workers. While I've gotten
try to be green and not throw up any instances of eval-usage by workers,
it is much safer to deploy this is Telemetry-only mode for Workers for
a little bit to see if anything pops up from the Nightly population.
Bug 1584602 is for enforcing the checks.
Differential Revision: https://phabricator.services.mozilla.com/D47480
--HG--
extra : moz-landing-system : lando
We log to MOZ_LOG, report an error to the console, send telemetry, and in debug builds - crash
Differential Revision: https://phabricator.services.mozilla.com/D45055
--HG--
extra : moz-landing-system : lando
We now correctly handle the following cases:
- Thunderbird
- the Browser Toolbox/Console
- Two safe and common idioms
- when general.config.filename is set and userChromeJS does shenanigans
We also change the function to only crash in Debug mode, and for Release channels
we report diagnostic information in a way that does not reveal user data.
Differential Revision: https://phabricator.services.mozilla.com/D39557
--HG--
extra : moz-landing-system : lando
1. Adding a new attribute chromeContext in ConsoleEvent
2. Adding a new boolean attribute isFromChromeContext in nsIConsoleMessage
3. Sending IsFromChromeContext to the parent process
Differential Revision: https://phabricator.services.mozilla.com/D23330
--HG--
extra : moz-landing-system : lando
1. Adding a new attribute chromeContext in ConsoleEvent
2. Adding a new boolean attribute isFromChromeContext in nsIConsoleMessage
3. Sending IsFromChromeContext to the parent process
Differential Revision: https://phabricator.services.mozilla.com/D23330
--HG--
extra : moz-landing-system : lando
Replaced instances of callers in both C++ and JS files to query the state from the principal directly.
Differential Revision: https://phabricator.services.mozilla.com/D22532
--HG--
extra : moz-landing-system : lando
Prevent web_accessible_resources resources loading in private contexts when extension does not have permission.
Differential Revision: https://phabricator.services.mozilla.com/D17138
--HG--
extra : moz-landing-system : lando
Prevent web_accessible_resources resources loading in private contexts when extension does not have permission.
Differential Revision: https://phabricator.services.mozilla.com/D17138
--HG--
extra : moz-landing-system : lando
Summary: Really sorry for the size of the patch. It's mostly automatic
s/nsIDocument/Document/ but I had to fix up in a bunch of places manually to
add the right namespacing and such.
Overall it's not a very interesting patch I think.
nsDocument.cpp turns into Document.cpp, nsIDocument.h into Document.h and
nsIDocumentInlines.h into DocumentInlines.h.
I also changed a bunch of nsCOMPtr usage to RefPtr, but not all of it.
While fixing up some of the bits I also removed some unneeded OwnerDoc() null
checks and such, but I didn't do anything riskier than that.
The security.allow_chrome_frames_inside_content pref was added to support
XUL extension compat after the hiddenWindow special case was removed in
Bug 1145470. Since we don't need to support that use-case anymore, this
changeset backs out the change that relanded the special case with the pref.
MozReview-Commit-ID: 4keMEIQvt1Y
--HG--
extra : rebase_source : 1a9c54c9807eaed2645d0ea03b5064ed7472d7a4
This interface is not usable from JS, because we don't expose initialize() in
the WebIDL bindings for Exception. And C++ doesn't use it.
MozReview-Commit-ID: LsIm4YA0YZE
There are several ways that expanded principals can be used as triggering
principals for requests. While that works fine for security checks, it also
sometimes causes them to be inherited, and used as result principals in
contexts where expanded principals aren't allowed.
This patch changes our inheritance behavior so that expanded principals are
downgraded to the most appropriate constituent principal when they would
otherwise be inherited.
The logic for choosing the most appropriate principal is a bit suspect, and
may eventually need to be changed to always select the last whitelist
principal, but I chose it to preserve the current principal downgrade behavior
used by XMLHttpRequest for the time being.
MozReview-Commit-ID: 9fvAKr2e2fa
--HG--
extra : rebase_source : c30df1b3851c11fed5a1d6a7fb158cec14933182
Right now, NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR expects singleton
constructors to return already-addrefed raw pointers, and while it accepts
constructors that return already_AddRefed, most existing don't do so.
Meanwhile, the convention elsewhere is that a raw pointer return value is
owned by the callee, and that the caller needs to addref it if it wants to
keep its own reference to it.
The difference in convention makes it easy to leak (I've definitely caused
more than one shutdown leak this way), so it would be better if we required
the singleton getters to return an explicit already_AddRefed, which would
behave the same for all callers.
This also cleans up several singleton constructors that left a dangling
pointer to their singletons when their initialization methods failed, when
they released their references without clearing their global raw pointers.
MozReview-Commit-ID: 9peyG4pRYcr
--HG--
extra : rebase_source : 2f5bd89c17cb554541be38444672a827c1392f3f
This class does some caching of an nsIClassInfo's flags and name. Presumably it
made sense at some time in the past, but now it's only used by
CanCreateWrapper(). It's simpler to just directly get the flags and name when
needed, so this patch does that and removes the class.
--HG--
extra : rebase_source : 0f11844ca6be4ff007b50d95bb8f5a88037fb1b5
This lets us replace moz_xstrdup() of string literals with AssignLiteral(),
among other improvements.
--HG--
extra : rebase_source : 9994d8ccb4f196cf63564b0dac2ae6c4370defb4
The current API makes the life time and ownership of the result array unclear
without careful reading. The result array is always owned by the principal,
and its lifetime tied to the lifetime of the principal itself. Returning a
const array reference makes this clear, and should prevent callers from
accidentally modifying the returned array.
MozReview-Commit-ID: 3f8mhynkKAj
--HG--
extra : source : 237acf2879f6222bc4b076c377bf026d18a6ebef
extra : amend_source : dfaf6e88e3c4758f7fdcf7fb422d457edafab1b7
The current API makes the life time and ownership of the result array unclear
without careful reading. The result array is always owned by the principal,
and its lifetime tied to the lifetime of the principal itself. Returning a
const array reference makes this clear, and should prevent callers from
accidentally modifying the returned array.
MozReview-Commit-ID: 3f8mhynkKAj
--HG--
extra : rebase_source : d2a5e0862f8c964fb5a3e46b50c2e9629b218699
extra : amend_source : 27d7a7ef5da6fe2aa1104009b6ee067465db73e1
This also introduces JS::GetObjectRealmOrNull, which returns an object's realm,
or null if the object is a cross-compartment wrapper. In the new order,
wrappers can't have realms, since they must be shared across all realms in a
compartment. We're introducing this new function early (even though it's
*currently* possible to assign a realm to wrappers) in order to see in
advance if the possibility of returning null will cause problems.
(It looks like it won't.)
--HG--
extra : rebase_source : e55ebbbc4edf2a18ce267198928246592060e339
extra : source : d6bfce1187aa13dbfab03f9566ff7b05b6705e70
In the new order, it will be a compartment-level bit rather than a
realm-level bit, so it does not belong on the Scope.
--HG--
extra : rebase_source : 44aa4620f7fd7f8d253c8c7f09bf8c97c00ff061
extra : source : 5a9c01720d7929e43aa70341d3821bfaa2479592
Going through the extension policy service rather than using
WebExtensionPolicy objects directly adds a lot of unnecessary overhead to
common operations on extension principals, and also makes the code more
complicated than it needs to be.
We also use weak references to policy objects here, since principals should
ideally lose as much of their elevated privileges as possible once the
extension instance that created them has been destroyed (which is something we
couldn't handle easily when we simply tracked ID strings).
MozReview-Commit-ID: KDNvVdvLkIt
--HG--
extra : rebase_source : 1b567919d2461bd0315d1a7d89f330cbd585f579
This is straightforward, with only two notable things.
- `#include "nsXPIDLString.h" is replaced with `#include "nsString.h"`
throughout, because all nsXPIDLString.h did was include nsString.h. The
exception is for files which already include nsString.h, in which case the
patch just removes the nsXPIDLString.h inclusion.
- The patch removes the |xpidl_string| gtest, but improves the |voided| test to
cover some of its ground, e.g. testing Adopt(nullptr).
--HG--
extra : rebase_source : 452cc4a08046a1adb1a8099a7e85a1917de5add8
This removes about 2/3 of the occurrences of nsXPIDLString in the tree. The
places where nsXPIDLStrings are null-checked are replaced with |rv| checks.
The patch also removes a couple of unused declarations from
nsIStringBundle.idl.
Note that nsStringBundle::GetStringFromNameHelper() was merged into
GetStringFromName(), because they both would have had the same signature.
--HG--
extra : rebase_source : ac40bc31c2a4997f2db0bd5069cc008757a2df6d
Most of the names passed to nsIStringBundle::{Get,Format}StringFromUTF8Name
have one of the two following forms:
- a 16-bit C string literal, which is then converted to an 8-bit string in
order for the lookup to occur;
- an 8-bit C string literal converted to a 16-bit string, which is then
converted back to an 8-bit string in order for the lookup to occur.
This patch introduces and uses alternative methods that can take an 8-bit C
string literal, which requires changing some signatures in other methods and
functions. It replaces all C++ uses of the old methods.
The patch also changes the existing {Get,Format}StringFromName() methods so
they take an AUTF8String argument for the name instead of a wstring, because
that's nicer for JS code.
Even though there is a method for C++ code and a different one for JS code,
|binaryname| is used so that the existing method names can be used for the
common case in both languages.
The change reduces the number of NS_ConvertUTF8toUTF16 and
NS_ConvertUTF16toUTF8 conversions while running Speedometer v2 from ~270,000 to
~160,000. (Most of these conversions involved the string
"deprecatedReferrerDirective" in nsCSPParser.cpp.)
--HG--
extra : rebase_source : 3bee57a501035f76a81230d95186f8c3f460ff8e
Our caller is C++ code, and the implementations are all also written in C++,
so there is no reason to go through SpiderMonkey here. This patch also makes
nsILoadContext builtinclass to ensure that the implementation is always native.
All the consumers relying on this have disappeared, so we can (finally!) get
rid of this ugly bit of special-casing.
MozReview-Commit-ID: HSeeG21O0p4
--HG--
extra : rebase_source : faebaf646308fe29b1c23e4117db2c1ea52ad658
Our caller is C++ code, and the implementations are all also written in C++,
so there is no reason to go through SpiderMonkey here. This patch also makes
nsILoadContext builtinclass to ensure that the implementation is always native.
Our caller is C++ code, and the implementations are all also written in C++,
so there is no reason to go through SpiderMonkey here. This patch also makes
nsILoadContext builtinclass to ensure that the implementation is always native.
