Bug 800915 - Reimplement PUNCTURE consumers in terms of isSafeToUnwrap() and remove PUNCTURE API. r=mrbkap

js/src/jswrapper.cpp

@@ -131,13 +131,11 @@ js::UnwrapOneChecked(JSContext *cx, HandleObject obj)
     }
 
     Wrapper *handler = Wrapper::wrapperHandler(obj);
-    bool rvOnFailure;
-    if (!handler->enter(cx, obj, JSID_VOID, Wrapper::PUNCTURE, &rvOnFailure))
-        return rvOnFailure ? (JSObject*) obj : NULL;
-    JSObject *ret = Wrapper::wrappedObject(obj);
-    JS_ASSERT(ret);
-
-    return ret;
+    if (!handler->isSafeToUnwrap()) {
+        JS_ReportError(cx, "Permission denied to access object");
+        return NULL;
+    }
+    return Wrapper::wrappedObject(obj);
 }
 
 bool
@@ -222,25 +220,22 @@ Wrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 }
 
 /*
- * Ordinarily, the convert trap would require a PUNCTURE. However, the default
+ * Ordinarily, the convert trap would require unwrapping. However, the default
  * implementation of convert, JS_ConvertStub, obtains a default value by calling
- * the toString/valueOf method on the wrapper, if any. Doing a PUNCTURE in this
- * case would be overly conservative. To make matters worse, XPConnect sometimes
+ * the toString/valueOf method on the wrapper, if any. Throwing if we can't unwrap
+ * in this case would be overly conservative. To make matters worse, XPConnect sometimes
  * installs a custom convert trap that obtains a default value by calling the
  * toString method on the wrapper. Doing a puncture in this case would be overly
- * conservative as well. We deal with these anomalies by clearing the pending
- * exception and falling back to the DefaultValue algorithm whenever the
- * PUNCTURE fails.
+ * conservative as well. We deal with these anomalies by falling back to the DefaultValue
+ * algorithm whenever unwrapping is forbidden.
  */
 bool
 Wrapper::defaultValue(JSContext *cx, JSObject *wrapper_, JSType hint, Value *vp)
 {
     RootedObject wrapper(cx, wrapper_);
 
-    bool status;
-    if (!enter(cx, wrapper_, JSID_VOID, PUNCTURE, &status)) {
+    if (!wrapperHandler(wrapper)->isSafeToUnwrap()) {
         RootedValue v(cx);
-        JS_ClearPendingException(cx);
         if (!DefaultValue(cx, wrapper, hint, &v))
             return false;
         *vp = v;

js/src/jswrapper.h

@@ -35,8 +35,7 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler
     enum Action {
         GET,
         SET,
-        CALL,
-        PUNCTURE
+        CALL
     };
 
     enum Flags {
@@ -72,24 +71,7 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler
      * on the underlying object's |id| property. In the case when |act| is CALL,
      * |id| is generally JSID_VOID.
      *
-     * The |act| parameter to enter() specifies the action being performed. GET,
-     * SET, and CALL are self-explanatory, but PUNCTURE requires more
-     * explanation:
-     *
-     * GET and SET allow for a very fine-grained security membrane, through
-     * which access can be granted or denied on a per-property, per-object, and
-     * per-action basis. Sometimes though, we just want to asks if we can access
-     * _everything_ behind the wrapper barrier. For example, when the structured
-     * clone algorithm runs up against a cross-compartment wrapper, it needs to
-     * know whether it can enter the compartment and keep cloning, or whether it
-     * should throw. This is the role of PUNCTURE.
-     *
-     * PUNCTURE allows the policy to specify whether the wrapper barrier may
-     * be lifted - that is to say, whether the caller is allowed to access
-     * anything that the wrapped object could access. This is a very powerful
-     * permission, and thus should generally be denied for security wrappers
-     * except under very special circumstances. When |act| is PUNCTURE, |id|
-     * should be JSID_VOID.
+     * The |act| parameter to enter() specifies the action being performed.
      */
     virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act,
                        bool *bp);

js/xpconnect/wrappers/AccessCheck.cpp

@@ -231,11 +231,6 @@ AccessCheck::isCrossOriginAccessPermitted(JSContext *cx, JSObject *wrapper, jsid
 
     JSObject *obj = Wrapper::wrappedObject(wrapper);
 
-    // PUNCTURE Is always denied for cross-origin access.
-    if (act == Wrapper::PUNCTURE) {
-        return false;
-    }
-
     const char *name;
     js::Class *clasp = js::GetObjectClass(obj);
     NS_ASSERTION(Jsvalify(clasp) != &XrayUtils::HolderClass, "shouldn't have a holder here");
@@ -345,9 +340,6 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper:
     if (act == Wrapper::CALL)
         return true;
 
-    if (act == Wrapper::PUNCTURE)
-        return false;
-
     jsid exposedPropsId = GetRTIdByIndex(cx, XPCJSRuntime::IDX_EXPOSEDPROPS);
 
     // We need to enter the wrappee's compartment to look at __exposedProps__,

js/xpconnect/wrappers/AccessCheck.h

@@ -96,9 +96,7 @@ struct LocationPolicy : public Policy {
         // We should only be dealing with Location objects here.
         MOZ_ASSERT(WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
 
-        // Location object security is complicated enough. Don't allow punctures.
-        if (act != js::Wrapper::PUNCTURE &&
-            (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act) ||
+        if ((AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act) ||
              AccessCheck::isLocationObjectSameOrigin(cx, wrapper))) {
             return true;
         }