Changeset 125146 in webkit


Ignore:
Timestamp:
Aug 8, 2012 8:52:41 PM (12 years ago)
Author:
abarth@webkit.org
Message:

Implement JSDOMWindow*::allowsAccessFrom* in terms of BindingSecurity
​https://bugs.webkit.org/show_bug.cgi?id=93407

Reviewed by Eric Seidel.

This patch removes allowsAccessFrom and implements the security checks
in terms of shouldAllowAccessToFrame directly. There shouldn't be any
change in behavior.

  • bindings/js/JSDOMWindowBase.cpp:

(WebCore):
(WebCore::shouldAllowAccessFrom):

  • bindings/js/JSDOMWindowBase.h:

(JSDOMWindowBase):

  • bindings/js/JSDOMWindowCustom.cpp:

(WebCore::namedItemGetter):
(WebCore::JSDOMWindow::getOwnPropertySlot):
(WebCore::JSDOMWindow::getOwnPropertyDescriptor):
(WebCore::JSDOMWindow::put):
(WebCore::JSDOMWindow::deleteProperty):
(WebCore::JSDOMWindow::getPropertyNames):
(WebCore::JSDOMWindow::getOwnPropertyNames):
(WebCore::JSDOMWindow::defineOwnProperty):
(WebCore::JSDOMWindow::setLocation):

  • bindings/js/JSDOMWindowCustom.h:
  • bindings/js/JSInjectedScriptManager.cpp:

(WebCore::InjectedScriptManager::canAccessInspectedWindow):

  • bindings/objc/WebScriptObject.mm:

(-[WebScriptObject _isSafeScript]):

  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateGetOwnPropertyDescriptorBody):
(GenerateImplementation):

  • bindings/scripts/test/JS/JSTestActiveDOMObject.cpp:

(WebCore::jsTestActiveDOMObjectExcitingAttr):
(WebCore::jsTestActiveDOMObjectConstructor):
(WebCore::jsTestActiveDOMObjectPrototypeFunctionExcitingFunction):

Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r125145 r125146  
     12012-08-08  Adam Barth  <abarth@webkit.org>
     2
     3        Implement JSDOMWindow*::allowsAccessFrom* in terms of BindingSecurity
     4        https://bugs.webkit.org/show_bug.cgi?id=93407
     5
     6        Reviewed by Eric Seidel.
     7
     8        This patch removes allowsAccessFrom and implements the security checks
     9        in terms of shouldAllowAccessToFrame directly. There shouldn't be any
     10        change in behavior.
     11
     12        * bindings/js/JSDOMWindowBase.cpp:
     13        (WebCore):
     14        (WebCore::shouldAllowAccessFrom):
     15        * bindings/js/JSDOMWindowBase.h:
     16        (JSDOMWindowBase):
     17        * bindings/js/JSDOMWindowCustom.cpp:
     18        (WebCore::namedItemGetter):
     19        (WebCore::JSDOMWindow::getOwnPropertySlot):
     20        (WebCore::JSDOMWindow::getOwnPropertyDescriptor):
     21        (WebCore::JSDOMWindow::put):
     22        (WebCore::JSDOMWindow::deleteProperty):
     23        (WebCore::JSDOMWindow::getPropertyNames):
     24        (WebCore::JSDOMWindow::getOwnPropertyNames):
     25        (WebCore::JSDOMWindow::defineOwnProperty):
     26        (WebCore::JSDOMWindow::setLocation):
     27        * bindings/js/JSDOMWindowCustom.h:
     28        * bindings/js/JSInjectedScriptManager.cpp:
     29        (WebCore::InjectedScriptManager::canAccessInspectedWindow):
     30        * bindings/objc/WebScriptObject.mm:
     31        (-[WebScriptObject _isSafeScript]):
     32        * bindings/scripts/CodeGeneratorJS.pm:
     33        (GenerateGetOwnPropertyDescriptorBody):
     34        (GenerateImplementation):
     35        * bindings/scripts/test/JS/JSTestActiveDOMObject.cpp:
     36        (WebCore::jsTestActiveDOMObjectExcitingAttr):
     37        (WebCore::jsTestActiveDOMObjectConstructor):
     38        (WebCore::jsTestActiveDOMObjectPrototypeFunctionExcitingFunction):
     39
    1402012-08-08  Sheriff Bot  <webkit.review.bot@gmail.com>
    241

  • trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp

    r123013 r125146  
    4343namespace WebCore {
    4444
     45// FIXME: This function should call shouldAllowAccessToDocument once there is a predictable
     46// way to get a Document* from a DOMWindow*.
     47static bool shouldAllowAccessFrom(const JSGlobalObject* thisObject, ExecState* exec)
     48{
     49    DOMWindow* active = activeDOMWindow(exec);
     50    DOMWindow* target = asJSDOMWindow(thisObject)->impl();
     51
     52    if (active->securityOrigin()->canAccess(target->securityOrigin()))
     53        return true;
     54
     55    printErrorMessageForFrame(target->frame(), target->crossDomainAccessErrorMessage(active));
     56    return false;
     57}
     58
    4559const ClassInfo JSDOMWindowBase::s_info = { "Window", &JSDOMGlobalObject::s_info, 0, 0, CREATE_METHOD_TABLE(JSDOMWindowBase) };
    4660
    47 const GlobalObjectMethodTable JSDOMWindowBase::s_globalObjectMethodTable = { &allowsAccessFrom, &supportsProfiling, &supportsRichSourceInfo, &shouldInterruptScript, &javaScriptExperimentsEnabled };
     61const GlobalObjectMethodTable JSDOMWindowBase::s_globalObjectMethodTable = { &shouldAllowAccessFrom, &supportsProfiling, &supportsRichSourceInfo, &shouldInterruptScript, &javaScriptExperimentsEnabled };
    4862
    4963JSDOMWindowBase::JSDOMWindowBase(JSGlobalData& globalData, Structure* structure, PassRefPtr<DOMWindow> window, JSDOMWindowShell* shell)
    … …  
    8498}
    8599
    86 String JSDOMWindowBase::crossDomainAccessErrorMessage(const JSGlobalObject* other) const
    87 {
    88     return m_shell->window()->impl()->crossDomainAccessErrorMessage(asJSDOMWindow(other)->impl());
    89 }
    90 
    91100void JSDOMWindowBase::printErrorMessage(const String& message) const
    92101{
    93102    printErrorMessageForFrame(impl()->frame(), message);
    94 }
    95 
    96 // This method checks whether accesss to *this* global object is permitted from
    97 // the given context; this differs from allowsAccessFromPrivate, since that
    98 // method checks whether the given context is permitted to access the current
    99 // window the shell is referencing (which may come from a different security
    100 // origin to this global object).
    101 bool JSDOMWindowBase::allowsAccessFrom(const JSGlobalObject* thisObject, ExecState* exec)
    102 {
    103     JSGlobalObject* otherObject = exec->lexicalGlobalObject();
    104 
    105     const JSDOMWindow* originWindow = asJSDOMWindow(otherObject);
    106     const JSDOMWindow* targetWindow = asJSDOMWindow(thisObject);
    107 
    108     if (originWindow == targetWindow)
    109         return true;
    110 
    111     const SecurityOrigin* originSecurityOrigin = originWindow->impl()->securityOrigin();
    112     const SecurityOrigin* targetSecurityOrigin = targetWindow->impl()->securityOrigin();
    113 
    114     if (originSecurityOrigin->canAccess(targetSecurityOrigin))
    115         return true;
    116 
    117     targetWindow->printErrorMessage(targetWindow->crossDomainAccessErrorMessage(otherObject));
    118     return false;
    119103}
    120104

  • trunk/Source/WebCore/bindings/js/JSDOMWindowBase.h

    r118018 r125146  
    6565        static bool shouldInterruptScript(const JSC::JSGlobalObject*);
    6666        static bool javaScriptExperimentsEnabled(const JSC::JSGlobalObject*);
    67         static bool allowsAccessFrom(const JSC::JSGlobalObject*, JSC::ExecState*);
    68        
    69         bool allowsAccessFrom(JSC::ExecState*) const;
    70         bool allowsAccessFromNoErrorMessage(JSC::ExecState*) const;
    71         bool allowsAccessFrom(JSC::ExecState*, String& message) const;
     67
    7268        void printErrorMessage(const String&) const;
    73 
    74         // Don't call this version of allowsAccessFrom -- it's a slightly incorrect implementation used only by WebScriptObject
    75         bool allowsAccessFrom(const JSC::JSGlobalObject*) const;
    7669       
    7770        static JSC::JSObject* toThisObject(JSC::JSCell*, JSC::ExecState*);
    … …  
    8376        RefPtr<DOMWindow> m_impl;
    8477        JSDOMWindowShell* m_shell;
    85 
    86         bool allowsAccessFromPrivate(const JSC::JSGlobalObject*) const;
    87         String crossDomainAccessErrorMessage(const JSC::JSGlobalObject*) const;
    8878    };
    8979

  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r122115 r125146  
    113113    Document* document = thisObj->impl()->frame()->document();
    114114
    115     ASSERT(thisObj->allowsAccessFrom(exec));
     115    ASSERT(shouldAllowAccessToFrame(exec, thisObj->impl()->frame()));
    116116    ASSERT(document);
    117117    ASSERT(document->isHTMLDocument());
    … …  
    159159    // is allowed.
    160160    String errorMessage;
    161     bool allowsAccess = thisObject->allowsAccessFrom(exec, errorMessage);
     161    bool allowsAccess = shouldAllowAccessToFrame(exec, thisObject->impl()->frame(), errorMessage);
    162162
    163163    // Look for overrides before looking at any of our own properties, but ignore overrides completely
    … …  
    167167
    168168    // We need this code here because otherwise JSDOMWindowBase will stop the search before we even get to the
    169     // prototype due to the blanket same origin (allowsAccessFrom) check at the end of getOwnPropertySlot.
     169    // prototype due to the blanket same origin check at the end of getOwnPropertySlot.
    170170    // Also, it's important to get the implementation straight out of the DOMWindow prototype regardless of
    171171    // what prototype is actually set on this object.
    … …  
    273273    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
    274274    // Never allow cross-domain getOwnPropertyDescriptor
    275     if (!thisObject->allowsAccessFrom(exec))
     275    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    276276        return false;
    277277
    … …  
    350350    // Optimization: access JavaScript global variables directly before involving the DOM.
    351351    if (thisObject->JSGlobalObject::hasOwnPropertyForWrite(exec, propertyName)) {
    352         if (thisObject->allowsAccessFrom(exec))
     352        if (shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    353353            JSGlobalObject::put(thisObject, exec, propertyName, value, slot);
    354354        return;
    … …  
    358358        return;
    359359
    360     if (thisObject->allowsAccessFrom(exec))
     360    if (shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    361361        Base::put(thisObject, exec, propertyName, value, slot);
    362362}
    … …  
    366366    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(cell);
    367367    // Only allow deleting properties by frames in the same origin.
    368     if (!thisObject->allowsAccessFrom(exec))
     368    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    369369        return false;
    370370    return Base::deleteProperty(thisObject, exec, propertyName);
    … …  
    375375    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
    376376    // Only allow the window to enumerated by frames in the same origin.
    377     if (!thisObject->allowsAccessFrom(exec))
     377    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    378378        return;
    379379    Base::getPropertyNames(thisObject, exec, propertyNames, mode);
    … …  
    384384    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
    385385    // Only allow the window to enumerated by frames in the same origin.
    386     if (!thisObject->allowsAccessFrom(exec))
     386    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    387387        return;
    388388    Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);
    … …  
    393393    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
    394394    // Only allow defining properties in this way by frames in the same origin, as it allows setters to be introduced.
    395     if (!thisObject->allowsAccessFrom(exec))
     395    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))
    396396        return false;
    397397
    … …  
    413413        if (Settings* settings = activeFrame->settings()) {
    414414            if (settings->usesDashboardBackwardCompatibilityMode() && !activeFrame->tree()->parent()) {
    415                 if (allowsAccessFrom(exec))
     415                if (shouldAllowAccessToFrame(exec, impl()->frame()))
    416416                    putDirect(exec->globalData(), Identifier(exec, "location"), value);
    417417                return;

  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.h

    r113387 r125146  
    3737}
    3838
    39 inline bool JSDOMWindowBase::allowsAccessFrom(const JSGlobalObject* other) const
    40 {
    41     if (allowsAccessFromPrivate(other))
    42         return true;
    43     printErrorMessage(crossDomainAccessErrorMessage(other));
    44     return false;
    45 }
    46 
    47 inline bool JSDOMWindowBase::allowsAccessFrom(JSC::ExecState* exec) const
    48 {
    49     if (allowsAccessFromPrivate(exec->lexicalGlobalObject()))
    50         return true;
    51     printErrorMessage(crossDomainAccessErrorMessage(exec->lexicalGlobalObject()));
    52     return false;
    53 }
    54    
    55 inline bool JSDOMWindowBase::allowsAccessFromNoErrorMessage(JSC::ExecState* exec) const
    56 {
    57     return allowsAccessFromPrivate(exec->lexicalGlobalObject());
    58 }
    59    
    60 inline bool JSDOMWindowBase::allowsAccessFrom(JSC::ExecState* exec, String& message) const
    61 {
    62     if (allowsAccessFromPrivate(exec->lexicalGlobalObject()))
    63         return true;
    64     message = crossDomainAccessErrorMessage(exec->lexicalGlobalObject());
    65     return false;
    66 }
    67    
    68 ALWAYS_INLINE bool JSDOMWindowBase::allowsAccessFromPrivate(const JSGlobalObject* other) const
    69 {
    70     const JSDOMWindow* originWindow = asJSDOMWindow(other);
    71     const JSDOMWindow* targetWindow = m_shell->window();
    72 
    73     if (originWindow == targetWindow)
    74         return true;
    75 
    76     const SecurityOrigin* originSecurityOrigin = originWindow->impl()->securityOrigin();
    77     const SecurityOrigin* targetSecurityOrigin = targetWindow->impl()->securityOrigin();
    78 
    79     return originSecurityOrigin->canAccess(targetSecurityOrigin);
    80 }
    81 
    8239}
    8340

  • trunk/Source/WebCore/bindings/js/JSInjectedScriptManager.cpp

    r121381 r125146  
    3737#include "InjectedScriptManager.h"
    3838
     39#include "BindingSecurity.h"
    3940#include "ExceptionCode.h"
    4041#include "JSDOMWindow.h"
    … …  
    8687    if (!inspectedWindow)
    8788        return false;
    88     return inspectedWindow->allowsAccessFromNoErrorMessage(scriptState);
     89    return BindingSecurity::shouldAllowAccessToFrame(scriptState, inspectedWindow->impl()->frame(), DoNotReportSecurityError);
    8990}
    9091

  • trunk/Source/WebCore/bindings/objc/WebScriptObject.mm

    r122635 r125146  
    242242        return false;
    243243
    244     return jsCast<JSDOMWindowBase*>(root->globalObject())->allowsAccessFrom(_private->originRootObject->globalObject());
     244    // It's not actually correct to call shouldAllowAccessToFrame in this way because
     245    // JSDOMWindowBase* isn't the right object to represent the currently executing
     246    // JavaScript. Instead, we should use ExecState, like we do elsewhere.
     247    JSDOMWindowBase* target = jsCast<JSDOMWindowBase*>(root->globalObject());
     248    return shouldAllowAccessToFrame(_private->originRootObject->globalObject()->globalExec(), target->impl()->frame());
    245249}
    246250

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r125042 r125146  
    486486    my @getOwnPropertyDescriptorImpl = ();
    487487    if ($dataNode->extendedAttributes->{"CheckSecurity"}) {
    488         if ($interfaceName eq "DOMWindow") {
    489             push(@implContent, "    if (!thisObject->allowsAccessFrom(exec))\n");
    490         } else {
    491             push(@implContent, "    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))\n");
    492         }
     488        push(@implContent, "    if (!shouldAllowAccessToFrame(exec, thisObject->impl()->frame()))\n");
    493489        push(@implContent, "        return false;\n");
    494490    }
    … …  
    17811777                    !$attribute->signature->extendedAttributes->{"DoNotCheckSecurity"} &&
    17821778                    !$attribute->signature->extendedAttributes->{"DoNotCheckSecurityOnGetter"}) {
    1783                     push(@implContent, "    if (!castedThis->allowsAccessFrom(exec))\n");
     1779                    push(@implContent, "    if (!shouldAllowAccessToFrame(exec, castedThis->impl()->frame()))\n");
    17841780                    push(@implContent, "        return jsUndefined();\n");
    17851781                }
    … …  
    18961892
    18971893                if ($dataNode->extendedAttributes->{"CheckSecurity"}) {
    1898                     push(@implContent, "    if (!domObject->allowsAccessFrom(exec))\n");
     1894                    push(@implContent, "    if (!shouldAllowAccessToFrame(exec, domObject->impl()->frame()))\n");
    18991895                    push(@implContent, "        return jsUndefined();\n");
    19001896                }
    … …  
    19701966
    19711967                            if ($dataNode->extendedAttributes->{"CheckSecurity"} && !$attribute->signature->extendedAttributes->{"DoNotCheckSecurity"}) {
    1972                                 if ($interfaceName eq "DOMWindow") {
    1973                                     push(@implContent, "    if (!jsCast<$className*>(thisObject)->allowsAccessFrom(exec))\n");
    1974                                 } else {
    1975                                     push(@implContent, "    if (!shouldAllowAccessToFrame(exec, jsCast<$className*>(thisObject)->impl()->frame()))\n");
    1976                                 }
     1968                                push(@implContent, "    if (!shouldAllowAccessToFrame(exec, jsCast<$className*>(thisObject)->impl()->frame()))\n");
    19771969                                push(@implContent, "        return;\n");
    19781970                            }
    … …  
    20992091                push(@implContent, "{\n");
    21002092                if ($dataNode->extendedAttributes->{"CheckSecurity"}) {
    2101                     if ($interfaceName eq "DOMWindow") {
    2102                         push(@implContent, "    if (!jsCast<$className*>(thisObject)->allowsAccessFrom(exec))\n");
    2103                     } else {
    2104                         push(@implContent, "    if (!shouldAllowAccessToFrame(exec, jsCast<$className*>(thisObject)->impl()->frame()))\n");
    2105                     }
     2093                    push(@implContent, "    if (!shouldAllowAccessToFrame(exec, jsCast<$className*>(thisObject)->impl()->frame()))\n");
    21062094                    push(@implContent, "        return;\n");
    21072095                }
    … …  
    22072195                if ($dataNode->extendedAttributes->{"CheckSecurity"} and
    22082196                    !$function->signature->extendedAttributes->{"DoNotCheckSecurity"}) {
    2209                     push(@implContent, "    if (!castedThis->allowsAccessFrom(exec))\n");
     2197                    push(@implContent, "    if (!shouldAllowAccessToFrame(exec, castedThis->impl()->frame()))\n");
    22102198                    push(@implContent, "        return JSValue::encode(jsUndefined());\n");
    22112199                }

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp

    r118651 r125146  
    154154{
    155155    JSTestActiveDOMObject* castedThis = jsCast<JSTestActiveDOMObject*>(asObject(slotBase));
    156     if (!castedThis->allowsAccessFrom(exec))
     156    if (!shouldAllowAccessToFrame(exec, castedThis->impl()->frame()))
    157157        return jsUndefined();
    158158    UNUSED_PARAM(exec);
    … …  
    166166{
    167167    JSTestActiveDOMObject* domObject = jsCast<JSTestActiveDOMObject*>(asObject(slotBase));
    168     if (!domObject->allowsAccessFrom(exec))
     168    if (!shouldAllowAccessToFrame(exec, domObject->impl()->frame()))
    169169        return jsUndefined();
    170170    return JSTestActiveDOMObject::getConstructor(exec, domObject->globalObject());
    … …  
    183183    JSTestActiveDOMObject* castedThis = jsCast<JSTestActiveDOMObject*>(asObject(thisValue));
    184184    ASSERT_GC_OBJECT_INHERITS(castedThis, &JSTestActiveDOMObject::s_info);
    185     if (!castedThis->allowsAccessFrom(exec))
     185    if (!shouldAllowAccessToFrame(exec, castedThis->impl()->frame()))
    186186        return JSValue::encode(jsUndefined());
    187187    TestActiveDOMObject* impl = static_cast<TestActiveDOMObject*>(castedThis->impl());
Note: See TracChangeset for help on using the changeset viewer.