Bug 1582857. Fix security checks around this-unwrapping. r=peterv,bholley

Differential Revision: https://phabricator.services.mozilla.com/D46692

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Boris Zbarsky 2019-09-24 01:02:25 +00:00
Родитель c3f9baa23f
Коммит 89dd3cf285
5 изменённых файлов: 135 добавлений и 22 удалений

Просмотреть файл

@ -54,6 +54,7 @@
#include "mozilla/dom/HTMLEmbedElement.h"
#include "mozilla/dom/HTMLElementBinding.h"
#include "mozilla/dom/HTMLEmbedElementBinding.h"
#include "mozilla/dom/MaybeCrossOriginObject.h"
#include "mozilla/dom/ReportingUtils.h"
#include "mozilla/dom/XULElementBinding.h"
#include "mozilla/dom/XULFrameElementBinding.h"
@ -2915,15 +2916,8 @@ struct MaybeGlobalThisPolicy : public NormalThisPolicy {
// We want the HandleInvalidThis of NormalThisPolicy.
};
// There are some LenientThis things on globals, so we inherit from
// MaybeGlobalThisPolicy.
struct LenientThisPolicy : public MaybeGlobalThisPolicy {
// We want the HasValidThisValue of MaybeGlobalThisPolicy.
// We want the ExtractThisObject of MaybeGlobalThisPolicy.
// We want the MaybeUnwrapThisObject of MaybeGlobalThisPolicy.
// Shared LenientThis behavior for our two different LenientThis policies.
struct LenientThisPolicyMixin {
static bool HandleInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
bool aSecurityError, prototypes::ID aProtoId) {
MOZ_ASSERT(!JS_IsExceptionPending(aCx));
@ -2935,6 +2929,20 @@ struct LenientThisPolicy : public MaybeGlobalThisPolicy {
}
};
// There are some LenientThis things on globals, so we inherit from
// MaybeGlobalThisPolicy.
struct LenientThisPolicy : public MaybeGlobalThisPolicy,
public LenientThisPolicyMixin {
// We want the HasValidThisValue of MaybeGlobalThisPolicy.
// We want the ExtractThisObject of MaybeGlobalThisPolicy.
// We want the MaybeUnwrapThisObject of MaybeGlobalThisPolicy.
// We want HandleInvalidThis from LenientThisPolicyMixin
using LenientThisPolicyMixin::HandleInvalidThis;
};
// There are some cross-origin things on globals, so we inherit from
// MaybeGlobalThisPolicy.
struct CrossOriginThisPolicy : public MaybeGlobalThisPolicy {
@ -2998,6 +3006,57 @@ struct CrossOriginThisPolicy : public MaybeGlobalThisPolicy {
// We want the HandleInvalidThis of MaybeGlobalThisPolicy.
};
// Some objects that can be cross-origin objects are globals, so we inherit
// from MaybeGlobalThisPolicy.
struct MaybeCrossOriginObjectThisPolicy : public MaybeGlobalThisPolicy {
// We want the HasValidThisValue of MaybeGlobalThisPolicy.
// We want the ExtractThisObject of MaybeGlobalThisPolicy.
// We want the MaybeUnwrapThisObject of MaybeGlobalThisPolicy
static MOZ_ALWAYS_INLINE nsresult UnwrapThisObject(
JS::MutableHandle<JSObject*> aObj, JSContext* aCx, void*& aSelf,
prototypes::ID aProtoID, uint32_t aProtoDepth) {
// There are two cases at this point: either aObj is a cross-compartment
// wrapper (CCW) or it's not. If it is, we don't need to do anything
// special compared to MaybeGlobalThisPolicy: the CCW will do the relevant
// security checks. Which is good, because if we tried to do the
// cross-origin object check _before_ unwrapping it would always come back
// as "same-origin" and if we tried to do it after unwrapping it would be
// completely wrong: the checks rely on the two sides of the comparison
// being symmetric (can access each other or cannot access each other), but
// if we have a CCW we could have an Xray, which is asymmetric. And then
// we'd think we should deny access, whereas we should actually allow
// access.
//
// If we do _not_ have a CCW here, then we need to check whether it's a
// cross-origin-accessible object, and if it is check whether it's
// same-origin-domain with our current callee.
if (!js::IsCrossCompartmentWrapper(aObj) &&
xpc::IsCrossOriginAccessibleObject(aObj) &&
!MaybeCrossOriginObjectMixins::IsPlatformObjectSameOrigin(aCx, aObj)) {
return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
}
return MaybeGlobalThisPolicy::UnwrapThisObject(aObj, aCx, aSelf, aProtoID,
aProtoDepth);
}
// We want the HandleInvalidThis of MaybeGlobalThisPolicy.
};
// And in some cases we are dealing with a maybe-cross-origin object _and_ need
// [LenientThis] behavior.
struct MaybeCrossOriginObjectLenientThisPolicy
: public MaybeCrossOriginObjectThisPolicy,
public LenientThisPolicyMixin {
// We want to get all of our behavior from
// MaybeCrossOriginObjectLenientThisPolicy, except for HandleInvalidThis,
// which should come from LenientThisPolicyMixin.
using LenientThisPolicyMixin::HandleInvalidThis;
};
/**
* An ExceptionPolicy struct provides a single HandleException method which is
* used to handle an exception, if any. The method is given the current
@ -3089,6 +3148,15 @@ template bool GenericGetter<CrossOriginThisPolicy, ThrowExceptions>(
JSContext* cx, unsigned argc, JS::Value* vp);
// There aren't any cross-origin Promise-returning getters, so don't
// bother instantiating that specialization.
template bool GenericGetter<MaybeCrossOriginObjectThisPolicy, ThrowExceptions>(
JSContext* cx, unsigned argc, JS::Value* vp);
// There aren't any maybe-cross-origin-object Promise-returning getters, so
// don't bother instantiating that specialization.
template bool GenericGetter<MaybeCrossOriginObjectLenientThisPolicy,
ThrowExceptions>(JSContext* cx, unsigned argc,
JS::Value* vp);
// There aren't any maybe-cross-origin-object Promise-returning lenient-this
// getters, so don't bother instantiating that specialization.
template <typename ThisPolicy>
bool GenericSetter(JSContext* cx, unsigned argc, JS::Value* vp) {
@ -3137,6 +3205,11 @@ template bool GenericSetter<LenientThisPolicy>(JSContext* cx, unsigned argc,
JS::Value* vp);
template bool GenericSetter<CrossOriginThisPolicy>(JSContext* cx, unsigned argc,
JS::Value* vp);
template bool GenericSetter<MaybeCrossOriginObjectThisPolicy>(JSContext* cx,
unsigned argc,
JS::Value* vp);
template bool GenericSetter<MaybeCrossOriginObjectLenientThisPolicy>(
JSContext* cx, unsigned argc, JS::Value* vp);
template <typename ThisPolicy, typename ExceptionPolicy>
bool GenericMethod(JSContext* cx, unsigned argc, JS::Value* vp) {
@ -3188,6 +3261,12 @@ template bool GenericMethod<CrossOriginThisPolicy, ThrowExceptions>(
JSContext* cx, unsigned argc, JS::Value* vp);
// There aren't any cross-origin Promise-returning methods, so don't
// bother instantiating that specialization.
template bool GenericMethod<MaybeCrossOriginObjectThisPolicy, ThrowExceptions>(
JSContext* cx, unsigned argc, JS::Value* vp);
template bool GenericMethod<MaybeCrossOriginObjectThisPolicy,
ConvertExceptionsToPromises>(JSContext* cx,
unsigned argc,
JS::Value* vp);
} // namespace binding_detail

Просмотреть файл

@ -2969,6 +2969,15 @@ struct LenientThisPolicy;
// A this-extraction policy for cross-origin getters/setters/methods.
struct CrossOriginThisPolicy;
// A this-extraction policy for getters/setters/methods that should
// not be allowed to be called cross-origin but expect objects that
// _can_ be cross-origin.
struct MaybeCrossOriginObjectThisPolicy;
// A this-extraction policy which is just like
// MaybeCrossOriginObjectThisPolicy but has lenient-this behavior.
struct MaybeCrossOriginObjectLenientThisPolicy;
// An exception-reporting policy for normal getters/setters/methods.
struct ThrowExceptions;

Просмотреть файл

@ -2564,6 +2564,9 @@ class MethodDefiner(PropertyDefiner):
if m.get("allowCrossOriginThis", False):
accessor = ("(GenericMethod<CrossOriginThisPolicy, %s>)" %
exceptionPolicy)
elif descriptor.interface.hasDescendantWithCrossOriginMembers:
accessor = ("(GenericMethod<MaybeCrossOriginObjectThisPolicy, %s>)" %
exceptionPolicy)
elif descriptor.interface.isOnGlobalProtoChain():
accessor = ("(GenericMethod<MaybeGlobalThisPolicy, %s>)" %
exceptionPolicy)
@ -2665,11 +2668,18 @@ class AttrDefiner(PropertyDefiner):
"readable attribute %s.%s" %
(self.descriptor.name,
attr.identifier.name))
if descriptor.interface.hasDescendantWithCrossOriginMembers:
accessor = ("GenericGetter<MaybeCrossOriginObjectLenientThisPolicy, %s>" %
exceptionPolicy)
else:
accessor = ("GenericGetter<LenientThisPolicy, %s>" %
exceptionPolicy)
elif attr.getExtendedAttribute("CrossOriginReadable"):
accessor = ("GenericGetter<CrossOriginThisPolicy, %s>" %
exceptionPolicy)
elif descriptor.interface.hasDescendantWithCrossOriginMembers:
accessor = ("GenericGetter<MaybeCrossOriginObjectThisPolicy, %s>" %
exceptionPolicy)
elif descriptor.interface.isOnGlobalProtoChain():
accessor = ("GenericGetter<MaybeGlobalThisPolicy, %s>" %
exceptionPolicy)
@ -2699,9 +2709,14 @@ class AttrDefiner(PropertyDefiner):
"writable attribute %s.%s" %
(descriptor.name,
attr.identifier.name))
if descriptor.interface.hasDescendantWithCrossOriginMembers:
accessor = "GenericSetter<MaybeCrossOriginObjectLenientThisPolicy>"
else:
accessor = "GenericSetter<LenientThisPolicy>"
elif attr.getExtendedAttribute("CrossOriginWritable"):
accessor = "GenericSetter<CrossOriginThisPolicy>"
elif descriptor.interface.hasDescendantWithCrossOriginMembers:
accessor = "GenericSetter<MaybeCrossOriginObjectThisPolicy>"
elif descriptor.interface.isOnGlobalProtoChain():
accessor = "GenericSetter<MaybeGlobalThisPolicy>"
else:

Просмотреть файл

@ -441,7 +441,6 @@ class Descriptor(DescriptorProvider):
if self.concrete:
self.proxy = False
self.hasCrossOriginMembers = False
iface = self.interface
for m in iface.members:
# Don't worry about inheriting legacycallers either: in
@ -456,17 +455,9 @@ class Descriptor(DescriptorProvider):
addOperation('LegacyCaller', m)
while iface:
for m in iface.members:
if (m.isAttr() and
(m.getExtendedAttribute("CrossOriginReadable") or
m.getExtendedAttribute("CrossOriginWritable"))):
self.hasCrossOriginMembers = True
if not m.isMethod():
continue
if m.getExtendedAttribute("CrossOriginCallable"):
self.hasCrossOriginMembers = True
def addIndexedOrNamedOperation(operation, m):
if m.isIndexed():
operation = 'Indexed' + operation
@ -725,7 +716,7 @@ class Descriptor(DescriptorProvider):
def isMaybeCrossOriginObject(self):
# If we're isGlobal and have cross-origin members, we're a Window, and
# that's not a cross-origin object. The WindowProxy is.
return self.concrete and self.hasCrossOriginMembers and not self.isGlobal()
return self.concrete and self.interface.hasCrossOriginMembers and not self.isGlobal()
def needsHeaderInclude(self):
"""

Просмотреть файл

@ -896,6 +896,10 @@ class IDLInterfaceOrNamespace(IDLInterfaceOrInterfaceMixinOrNamespace):
# If this is an iterator interface, we need to know what iterable
# interface we're iterating for in order to get its nativeType.
self.iterableInterface = None
# True if we have cross-origin members.
self.hasCrossOriginMembers = False
# True if some descendant (including ourselves) has cross-origin members
self.hasDescendantWithCrossOriginMembers = False
self.toStringTag = toStringTag
@ -1199,6 +1203,21 @@ class IDLInterfaceOrNamespace(IDLInterfaceOrInterfaceMixinOrNamespace):
not hasattr(member, "originatingInterface")):
member.originatingInterface = self
for member in self.members:
if ((member.isMethod() and
member.getExtendedAttribute("CrossOriginCallable")) or
(member.isAttr() and
(member.getExtendedAttribute("CrossOriginReadable") or
member.getExtendedAttribute("CrossOriginWritable")))):
self.hasCrossOriginMembers = True
break
if self.hasCrossOriginMembers:
parent = self
while parent:
parent.hasDescendantWithCrossOriginMembers = True
parent = parent.parent
# Compute slot indices for our members before we pull in unforgeable
# members from our parent. Also, maplike/setlike declarations get a
# slot to hold their backing object.