mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 805807 - Rearchitect filtering policies so that check() doesn't throw on denial. r=mrbkap 

This is another one of those annoying situaitons in XPConnect right now where we
can't ask a question without potentially throwing if the answer is no. There's
also a bunch of unused cruft in here (like the Perm*Access stuff), so this stuff
was ripe for a spring cleaning. Unfortunately, I wasn't able to divide this patch
up nicely. Sorry for the big diff. :-(

In a nutshell, this patch changes things so that Policy::check() just becomes
a predicate that says whether the access is allowed or not. There's the remote
possibility that one of the underlying JSAPI calls in a ::check() implementation
might throw, so callers to ::check() should check JS_IsExceptionPending
afterwards (this doesn't catch OOM, but we can just continue along until the
next OOM-triggering operation and throw there).

Aside from exceptional cases, callers should call Policy::deny if they want to
report the failure. Policy::deny returns success value that should be returned
to the wrapper's consumer.
This commit is contained in:
Bobby Holley 2012-11-02 21:47:49 -03:00
Родитель 6c9faad2ad
Коммит 2669b93e81
4 изменённых файлов: 51 добавлений и 107 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

6
js/src/jswrapper.h
Просмотреть файл

@ -43,12 +43,6 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler
LAST_USED_FLAG = CROSS_COMPARTMENT
};
typedef enum {
PermitObjectAccess,
PermitPropertyAccess,
DenyAccess
} Permission;
static JSObject *New(JSContext *cx, JSObject *obj, JSObject *proto,
JSObject *parent, Wrapper *handler);

55
js/xpconnect/wrappers/AccessCheck.cpp
Просмотреть файл

@ -329,17 +329,6 @@ AccessCheck::deny(JSContext *cx, jsid id)
enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 };
static bool
Deny(JSContext *cx, jsid id, Wrapper::Action act)
{
// Refuse to perform the action and just return the default value.
if (act == Wrapper::GET)
return true;
// If its a set, deny it and throw an exception.
AccessCheck::deny(cx, id);
return false;
}
static bool
IsInSandbox(JSContext *cx, JSObject *obj)
{
@ -349,19 +338,15 @@ IsInSandbox(JSContext *cx, JSObject *obj)
}
bool
ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act,
Permission &perm)
ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act)
{
JSObject *wrappedObject = Wrapper::wrappedObject(wrapper);
if (act == Wrapper::CALL) {
perm = PermitObjectAccess;
if (act == Wrapper::CALL)
return true;
}
perm = DenyAccess;
if (act == Wrapper::PUNCTURE)
return Deny(cx, id, act);
return false;
jsid exposedPropsId = GetRTIdByIndex(cx, XPCJSRuntime::IDX_EXPOSEDPROPS);
@ -380,7 +365,6 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper:
JS_IsTypedArrayObject(wrappedObject, cx)) &&
((JSID_IS_INT(id) && JSID_TO_INT(id) >= 0) ||
(JSID_IS_STRING(id) && JS_FlatStringEqualsAscii(JSID_TO_FLAT_STRING(id), "length")))) {
perm = PermitPropertyAccess;
return true; // Allow
}
@ -405,26 +389,20 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper:
}
}
perm = PermitPropertyAccess;
return true;
}
return Deny(cx, id, act);
return false;
}
if (id == JSID_VOID) {
// This will force the caller to call us back for individual property accesses.
perm = PermitPropertyAccess;
if (id == JSID_VOID)
return true;
}
JS::Value exposedProps;
if (!JS_LookupPropertyById(cx, wrappedObject, exposedPropsId, &exposedProps))
return false;
if (exposedProps.isNullOrUndefined()) {
JSAutoCompartment wrapperAC(cx, wrapper);
return Deny(cx, id, act);
}
if (exposedProps.isNullOrUndefined())
return false;
if (!exposedProps.isObject()) {
JS_ReportError(cx, "__exposedProps__ must be undefined, null, or an Object");
@ -440,10 +418,8 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper:
if (!JS_GetPropertyDescriptorById(cx, hallpass, id, JSRESOLVE_QUALIFIED, &desc)) {
return false; // Error
}
if (desc.obj == NULL || !(desc.attrs & JSPROP_ENUMERATE)) {
JSAutoCompartment wrapperAC(cx, wrapper);
return Deny(cx, id, act);
}
if (!desc.obj || !(desc.attrs & JSPROP_ENUMERATE))
return false;
if (!JSVAL_IS_STRING(desc.value)) {
JS_ReportError(cx, "property must be a string");
@ -487,19 +463,15 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper:
if ((act == Wrapper::SET && !(access & WRITE)) ||
(act != Wrapper::SET && !(access & READ))) {
JSAutoCompartment wrapperAC(cx, wrapper);
return Deny(cx, id, act);
return false;
}
perm = PermitPropertyAccess;
return true; // Allow
return true;
}
bool
ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act,
Permission &perm)
ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act)
{
perm = DenyAccess;
JSAutoCompartment ac(cx, wrapper);
if (JSID_IS_STRING(id) && act == Wrapper::GET) {
@ -510,7 +482,6 @@ ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper
JS_FlatStringEqualsAscii(flatId, "interfacesByID") ||
JS_FlatStringEqualsAscii(flatId, "results"))
{
perm = PermitPropertyAccess;
return true;
}
}
@ -519,11 +490,9 @@ ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper
// so we need this dynamic check. This can go away when we expose Components
// as SpecialPowers.wrap(Components) during automation.
if (xpc::IsUniversalXPConnectEnabled(cx)) {
perm = PermitPropertyAccess;
return true;
}
AccessCheck::deny(cx, id);
return false;
}

72
js/xpconnect/wrappers/AccessCheck.h
Просмотреть файл

@ -39,33 +39,16 @@ class AccessCheck {
};
struct Policy {
typedef js::Wrapper::Permission Permission;
static const Permission PermitObjectAccess = js::Wrapper::PermitObjectAccess;
static const Permission PermitPropertyAccess = js::Wrapper::PermitPropertyAccess;
static const Permission DenyAccess = js::Wrapper::DenyAccess;
};
// This policy permits access to all properties.
struct Permissive : public Policy {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
Permission &perm) {
perm = PermitObjectAccess;
return true;
}
};
// This policy only permits access to the object if the subject can touch
// system objects.
struct OnlyIfSubjectIsSystem : public Policy {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
Permission &perm) {
if (AccessCheck::isSystemOnlyAccessPermitted(cx)) {
perm = PermitObjectAccess;
return true;
}
perm = DenyAccess;
JSAutoCompartment ac(cx, wrapper);
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
return AccessCheck::isSystemOnlyAccessPermitted(cx);
}
static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
AccessCheck::deny(cx, id);
return false;
}
@ -74,17 +57,12 @@ struct OnlyIfSubjectIsSystem : public Policy {
// This policy only permits access to properties that are safe to be used
// across origins.
struct CrossOriginAccessiblePropertiesOnly : public Policy {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
Permission &perm) {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
// Location objects should always use LocationPolicy.
MOZ_ASSERT(!WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
if (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act)) {
perm = PermitPropertyAccess;
return true;
}
perm = DenyAccess;
JSAutoCompartment ac(cx, wrapper);
return AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act);
}
static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
AccessCheck::deny(cx, id);
return false;
}
@ -114,23 +92,19 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy {
// state of the outer window to determine whether we happen to be same-origin
// at the moment.
struct LocationPolicy : public Policy {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
Permission &perm) {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
// We should only be dealing with Location objects here.
MOZ_ASSERT(WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
// Default to deny.
perm = DenyAccess;
// Location object security is complicated enough. Don't allow punctures.
if (act != js::Wrapper::PUNCTURE &&
(AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act) ||
AccessCheck::isLocationObjectSameOrigin(cx, wrapper))) {
perm = PermitPropertyAccess;
return true;
}
JSAutoCompartment ac(cx, wrapper);
return false;
}
static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
AccessCheck::deny(cx, id);
return false;
}
@ -139,14 +113,26 @@ struct LocationPolicy : public Policy {
// This policy only permits access to properties if they appear in the
// objects exposed properties list.
struct ExposedPropertiesOnly : public Policy {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
Permission &perm);
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
// For gets, silently fail.
if (act == js::Wrapper::GET)
return true;
// For sets,throw an exception.
AccessCheck::deny(cx, id);
return false;
}
};
// Components specific policy
struct ComponentsObjectPolicy : public Policy {
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
Permission &perm);
static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
AccessCheck::deny(cx, id);
return false;
}
};
}

25
js/xpconnect/wrappers/FilteringWrapper.cpp
Просмотреть файл

@ -29,12 +29,6 @@ FilteringWrapper<Base, Policy>::~FilteringWrapper()
{
}
typedef Wrapper::Permission Permission;
static const Permission PermitObjectAccess = Wrapper::PermitObjectAccess;
static const Permission PermitPropertyAccess = Wrapper::PermitPropertyAccess;
static const Permission DenyAccess = Wrapper::DenyAccess;
template <typename Policy>
static bool
Filter(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
@ -42,11 +36,10 @@ Filter(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
size_t w = 0;
for (size_t n = 0; n < props.length(); ++n) {
jsid id = props[n];
Permission perm;
if (!Policy::check(cx, wrapper, id, Wrapper::GET, perm))
return false; // Error
if (perm != DenyAccess)
if (Policy::check(cx, wrapper, id, Wrapper::GET))
props[w++] = id;
else if (JS_IsExceptionPending(cx))
return false;
}
props.resize(w);
return true;
@ -92,14 +85,16 @@ bool
FilteringWrapper<Base, Policy>::enter(JSContext *cx, JSObject *wrapper, jsid id,
Wrapper::Action act, bool *bp)
{
Permission perm;
if (!Policy::check(cx, wrapper, id, act, perm)) {
*bp = false;
if (!Policy::check(cx, wrapper, id, act)) {
if (JS_IsExceptionPending(cx)) {
*bp = false;
return false;
}
JSAutoCompartment ac(cx, wrapper);
*bp = Policy::deny(cx, id, act);
return false;
}
*bp = true;
if (perm == DenyAccess)
return false;
return Base::enter(cx, wrapper, id, act, bp);
}