diff --git a/js/src/js.msg b/js/src/js.msg index 7b13a1cda096..c72d09521885 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -79,8 +79,8 @@ MSG_DEF(JSMSG_TOO_DEEP, 25, 1, JSEXN_INTERNALERR, "{0} nested too MSG_DEF(JSMSG_OVER_RECURSED, 26, 0, JSEXN_INTERNALERR, "too much recursion") MSG_DEF(JSMSG_IN_NOT_OBJECT, 27, 1, JSEXN_TYPEERR, "invalid 'in' operand {0}") MSG_DEF(JSMSG_BAD_NEW_RESULT, 28, 1, JSEXN_TYPEERR, "invalid new expression result {0}") -MSG_DEF(JSMSG_UNUSED29, 29, 0, JSEXN_NONE, "") -MSG_DEF(JSMSG_UNUSED30, 30, 0, JSEXN_NONE, "") +MSG_DEF(JSMSG_OBJECT_ACCESS_DENIED, 29, 0, JSEXN_ERR, "Permission denied to access object") +MSG_DEF(JSMSG_PROPERTY_ACCESS_DENIED, 30, 1, JSEXN_ERR, "Permission denied to access property '{0}'") MSG_DEF(JSMSG_BAD_INSTANCEOF_RHS, 31, 1, JSEXN_TYPEERR, "invalid 'instanceof' operand {0}") MSG_DEF(JSMSG_BAD_BYTECODE, 32, 1, JSEXN_INTERNALERR, "unimplemented JavaScript bytecode {0}") MSG_DEF(JSMSG_BAD_RADIX, 33, 0, JSEXN_RANGEERR, "radix must be an integer at least 2 and no greater than 36") diff --git a/js/src/jsproxy.cpp b/js/src/jsproxy.cpp index daf9f1c397e6..ebdc57ba827c 100644 --- a/js/src/jsproxy.cpp +++ b/js/src/jsproxy.cpp @@ -49,6 +49,20 @@ GetFunctionProxyConstruct(UnrootedObject proxy) return proxy->getSlotRef(JSSLOT_PROXY_CONSTRUCT); } +void +js::AutoEnterPolicy::reportError(JSContext *cx, jsid id) +{ + if (JSID_IS_VOID(id)) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, + JSMSG_OBJECT_ACCESS_DENIED); + } else { + JSString *str = IdToString(cx, id); + const jschar *prop = str ? str->getCharsZ(cx) : NULL; + JS_ReportErrorNumberUC(cx, js_GetErrorMessage, NULL, + JSMSG_PROPERTY_ACCESS_DENIED, prop); + } +} + BaseProxyHandler::BaseProxyHandler(void *family) : mFamily(family), mHasPrototype(false), diff --git a/js/src/jsproxy.h b/js/src/jsproxy.h index 0d0b70e1e8b8..315fb9b01ebc 100644 --- a/js/src/jsproxy.h +++ b/js/src/jsproxy.h @@ -84,6 +84,8 @@ class JS_FRIEND_API(BaseProxyHandler) { * generally JSID_VOID. * * The |act| parameter to enter() specifies the action being performed. + * If |bp| is false, the trap suggests that the caller throw (though it + * may still decide to squelch the error). */ enum Action { GET, @@ -341,6 +343,28 @@ NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv, JSObject * RenewProxyObject(JSContext *cx, JSObject *obj, BaseProxyHandler *handler, Value priv); +class JS_FRIEND_API(AutoEnterPolicy) +{ + public: + typedef BaseProxyHandler::Action Action; + AutoEnterPolicy(JSContext *cx, BaseProxyHandler *handler, + JSObject *wrapper, jsid id, Action act, bool mayThrow) + { + allow = handler->hasPolicy() ? handler->enter(cx, wrapper, id, act, &rv) + : true; + if (!allow && !rv && mayThrow) + reportError(cx, id); + } + + inline bool allowed() { return allow; } + inline bool returnValue() { JS_ASSERT(!allowed()); return rv; } + + protected: + void reportError(JSContext *cx, jsid id); + bool allow; + bool rv; +}; + } /* namespace js */ extern JS_FRIEND_API(JSObject *) diff --git a/js/src/jswrapper.cpp b/js/src/jswrapper.cpp index 2fc1c947f07f..0d4899e8d3f8 100644 --- a/js/src/jswrapper.cpp +++ b/js/src/jswrapper.cpp @@ -117,9 +117,9 @@ js::IsCrossCompartmentWrapper(RawObject wrapper) #define CHECKED(op, act) \ JS_BEGIN_MACRO \ - bool status; \ - if (!enter(cx, wrapper, id, act, &status)) \ - return status; \ + AutoEnterPolicy policy(cx, this, wrapper, id, act, true); \ + if (!policy.allowed()) \ + return policy.returnValue(); \ return (op); \ JS_END_MACRO diff --git a/js/xpconnect/wrappers/AccessCheck.cpp b/js/xpconnect/wrappers/AccessCheck.cpp index d5072d874990..3865c4aeb0f3 100644 --- a/js/xpconnect/wrappers/AccessCheck.cpp +++ b/js/xpconnect/wrappers/AccessCheck.cpp @@ -267,24 +267,6 @@ AccessCheck::isScriptAccessOnly(JSContext *cx, JSObject *wrapper) return false; } -void -AccessCheck::deny(JSContext *cx, jsid id) -{ - if (id == JSID_VOID) { - JS_ReportError(cx, "Permission denied to access object"); - } else { - jsval idval; - if (!JS_IdToValue(cx, id, &idval)) - return; - JSString *str = JS_ValueToString(cx, idval); - if (!str) - return; - const jschar *chars = JS_GetStringCharsZ(cx, str); - if (chars) - JS_ReportError(cx, "Permission denied to access property '%hs'", chars); - } -} - enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 }; static bool diff --git a/js/xpconnect/wrappers/AccessCheck.h b/js/xpconnect/wrappers/AccessCheck.h index 8a7dcc5a5b45..dd95e0407316 100644 --- a/js/xpconnect/wrappers/AccessCheck.h +++ b/js/xpconnect/wrappers/AccessCheck.h @@ -33,8 +33,6 @@ class AccessCheck { static bool needsSystemOnlyWrapper(JSObject *obj); static bool isScriptAccessOnly(JSContext *cx, JSObject *wrapper); - - static void deny(JSContext *cx, jsid id); }; struct Policy { @@ -45,8 +43,7 @@ struct Opaque : public Policy { static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) { return act == js::Wrapper::CALL; } - static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { - AccessCheck::deny(cx, id); + static bool deny(js::Wrapper::Action act) { return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) @@ -62,8 +59,7 @@ struct OnlyIfSubjectIsSystem : public Policy { return AccessCheck::isSystemOnlyAccessPermitted(cx); } - static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { - AccessCheck::deny(cx, id); + static bool deny(js::Wrapper::Action act) { return false; } @@ -79,8 +75,7 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy { static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) { return AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act); } - static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { - AccessCheck::deny(cx, id); + static bool deny(js::Wrapper::Action act) { return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) @@ -94,13 +89,9 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy { struct ExposedPropertiesOnly : public Policy { 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; + static bool deny(js::Wrapper::Action act) { + // Fail silently for GETs. + return act == js::Wrapper::GET; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl); }; @@ -109,8 +100,7 @@ struct ExposedPropertiesOnly : public Policy { struct ComponentsObjectPolicy : public Policy { 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); + static bool deny(js::Wrapper::Action act) { return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) { diff --git a/js/xpconnect/wrappers/FilteringWrapper.cpp b/js/xpconnect/wrappers/FilteringWrapper.cpp index fd4e81de9e58..69c5a976b484 100644 --- a/js/xpconnect/wrappers/FilteringWrapper.cpp +++ b/js/xpconnect/wrappers/FilteringWrapper.cpp @@ -146,12 +146,7 @@ FilteringWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, return true; } 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); + *bp = JS_IsExceptionPending(cx) ? false : Policy::deny(act); return false; } *bp = true;