Changeset 126249 in webkit


Ignore:
Timestamp:
Aug 21, 2012 6:50:40 PM (12 years ago)
Author:
abarth@webkit.org
Message:

V8 shouldn't have its own way of printing cross-origin error messages
​https://bugs.webkit.org/show_bug.cgi?id=94641

Reviewed by Eric Seidel.

Source/WebCore:

V8 used to re-implement (poorly) the code for printing out an error
message when a same-origin check failed. This patch deletes that code
in favor of just calling the WebCore version of the code. There more to
clean up here, but I had to stop before spidering over the whole
codebase.

  • bindings/generic/BindingSecurity.cpp:

(WebCore::canAccessDocument):

  • bindings/js/BindingState.cpp:
  • bindings/js/BindingState.h:
  • bindings/v8/BindingState.cpp:

(WebCore::printErrorMessageForFrame):

  • bindings/v8/BindingState.h:

(WebCore):

  • bindings/v8/V8DOMWindowShell.cpp:

(WebCore::reportUnsafeJavaScriptAccess):

  • bindings/v8/V8Proxy.cpp:
  • bindings/v8/V8Proxy.h:

(V8Proxy):

LayoutTests:

Update these results to reflect the new error messages. These error
messages are both more correct and more like JavaScriptCore.

  • platform/chromium/http/tests/security/cross-frame-access-private-browsing-expected.txt: Added.
    • We don't use the private browsing setting to implement private browsing.
  • platform/chromium/http/tests/security/cross-frame-access-document-direct-expected.txt:
  • platform/chromium/http/tests/security/inactive-document-with-empty-security-origin-expected.txt:
  • platform/chromium/http/tests/security/listener/xss-inactive-closure-expected.txt:
  • platform/chromium/http/tests/security/xss-eval-expected.txt:
    • Previously, we were incorrectly using the first script rather than the active script when printing the error message.
  • platform/chromium/http/tests/security/listener/xss-JSTargetNode-onclick-addEventListener-expected.txt: Removed.
  • platform/chromium/http/tests/security/listener/xss-JSTargetNode-onclick-shortcut-expected.txt: Removed.
  • platform/chromium/http/tests/security/listener/xss-XMLHttpRequest-addEventListener-expected.txt: Removed.
  • platform/chromium/http/tests/security/listener/xss-XMLHttpRequest-shortcut-expected.txt: Removed.
  • platform/chromium/http/tests/security/listener/xss-window-onclick-addEventListener-expected.txt: Removed.
  • platform/chromium/http/tests/security/listener/xss-window-onclick-shortcut-expected.txt: Removed.
    • These results are now identical to JSC.
Location:
trunk
Files:
1 added
6 deleted
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r126248 r126249  
     12012-08-21  Adam Barth  <abarth@webkit.org>
     2
     3        V8 shouldn't have its own way of printing cross-origin error messages
     4        https://bugs.webkit.org/show_bug.cgi?id=94641
     5
     6        Reviewed by Eric Seidel.
     7
     8        Update these results to reflect the new error messages. These error
     9        messages are both more correct and more like JavaScriptCore.
     10
     11        * platform/chromium/http/tests/security/cross-frame-access-private-browsing-expected.txt: Added.
     12            - We don't use the private browsing setting to implement private browsing.
     13        * platform/chromium/http/tests/security/cross-frame-access-document-direct-expected.txt:
     14        * platform/chromium/http/tests/security/inactive-document-with-empty-security-origin-expected.txt:
     15        * platform/chromium/http/tests/security/listener/xss-inactive-closure-expected.txt:
     16        * platform/chromium/http/tests/security/xss-eval-expected.txt:
     17            - Previously, we were incorrectly using the first script rather
     18              than the active script when printing the error message.
     19        * platform/chromium/http/tests/security/listener/xss-JSTargetNode-onclick-addEventListener-expected.txt: Removed.
     20        * platform/chromium/http/tests/security/listener/xss-JSTargetNode-onclick-shortcut-expected.txt: Removed.
     21        * platform/chromium/http/tests/security/listener/xss-XMLHttpRequest-addEventListener-expected.txt: Removed.
     22        * platform/chromium/http/tests/security/listener/xss-XMLHttpRequest-shortcut-expected.txt: Removed.
     23        * platform/chromium/http/tests/security/listener/xss-window-onclick-addEventListener-expected.txt: Removed.
     24        * platform/chromium/http/tests/security/listener/xss-window-onclick-shortcut-expected.txt: Removed.
     25            - These results are now identical to JSC.
     26
    1272012-08-21  Shinya Kawanaka  <shinyak@chromium.org>
    228

  • trunk/LayoutTests/platform/chromium/http/tests/security/cross-frame-access-document-direct-expected.txt

    r104803 r126249  
    1 CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-document-direct-test-victim.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-document-direct.html. Domains, protocols and ports must match.
     1CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-document-direct-test-victim.html from frame with URL http://127.0.0.1:8000/security/resources/cross-frame-iframe-for-document-direct-test.html. Domains, protocols and ports must match.
    22
    33Test cross-origin direct document access.

  • trunk/LayoutTests/platform/chromium/http/tests/security/inactive-document-with-empty-security-origin-expected.txt

    r104803 r126249  
    1 CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL about:blank from frame with URL http://127.0.0.1:8000/security/inactive-document-with-empty-security-origin.html. Domains, protocols and ports must match.
     1CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL about:blank from frame with URL http://127.0.0.1:8000/security/inactive-document-with-empty-security-origin.html#stop. Domains, protocols and ports must match.
    22
    33This test passes if it doesn't alert something ugly.

  • trunk/LayoutTests/platform/chromium/http/tests/security/listener/xss-inactive-closure-expected.txt

    r106765 r126249  
    1 CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/listener/resources/xss-inactive-closure-child-2.html from frame with URL http://127.0.0.1:8000/security/listener/resources/childWithButton.html. Domains, protocols and ports must match.
     1CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/listener/resources/xss-inactive-closure-child-2.html from frame with URL http://127.0.0.1:8000/security/listener/resources/xss-inactive-closure-child.html. Domains, protocols and ports must match.
    22
    33This tests that when a frame navigates to a new page, closures in the old page cannot access page content of the new page if there are from different domains.

  • trunk/LayoutTests/platform/chromium/http/tests/security/xss-eval-expected.txt

    r106736 r126249  
    1 CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/xss-eval3.html from frame with URL http://127.0.0.1:8000/security/xss-eval.html. Domains, protocols and ports must match.
     1CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/xss-eval3.html from frame with URL http://127.0.0.1:8000/security/resources/xss-eval2.html. Domains, protocols and ports must match.
    22
    33This page verifies that you can't use eval to subvert cross-domain checks.

  • trunk/Source/WebCore/ChangeLog

    r126248 r126249  
     12012-08-21  Adam Barth  <abarth@webkit.org>
     2
     3        V8 shouldn't have its own way of printing cross-origin error messages
     4        https://bugs.webkit.org/show_bug.cgi?id=94641
     5
     6        Reviewed by Eric Seidel.
     7
     8        V8 used to re-implement (poorly) the code for printing out an error
     9        message when a same-origin check failed. This patch deletes that code
     10        in favor of just calling the WebCore version of the code. There more to
     11        clean up here, but I had to stop before spidering over the whole
     12        codebase.
     13
     14        * bindings/generic/BindingSecurity.cpp:
     15        (WebCore::canAccessDocument):
     16        * bindings/js/BindingState.cpp:
     17        * bindings/js/BindingState.h:
     18        * bindings/v8/BindingState.cpp:
     19        (WebCore::printErrorMessageForFrame):
     20        * bindings/v8/BindingState.h:
     21        (WebCore):
     22        * bindings/v8/V8DOMWindowShell.cpp:
     23        (WebCore::reportUnsafeJavaScriptAccess):
     24        * bindings/v8/V8Proxy.cpp:
     25        * bindings/v8/V8Proxy.h:
     26        (V8Proxy):
     27
    1282012-08-21  Shinya Kawanaka  <shinyak@chromium.org>
    229

  • trunk/Source/WebCore/bindings/generic/BindingSecurity.cpp

    r126165 r126249  
    5656
    5757    if (reportingOption == ReportSecurityError)
    58         immediatelyReportUnsafeAccessTo(state, targetDocument);
     58        printErrorMessageForFrame(targetDocument->frame(), targetDocument->domWindow()->crossDomainAccessErrorMessage(active));
    5959
    6060    return false;

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

    r125126 r126249  
    4848}
    4949
    50 void immediatelyReportUnsafeAccessTo(ExecState* exec, Document* target)
    51 {
    52     printErrorMessageForFrame(target->frame(), target->domWindow()->crossDomainAccessErrorMessage(activeDOMWindow(exec)));
    5350}
    54 
    55 }

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

    r125126 r126249  
    4949inline Frame* firstFrame(BindingState*) { return 0; }
    5050
    51 void immediatelyReportUnsafeAccessTo(BindingState*, Document* target);
    52 
    5351}
    5452

  • trunk/Source/WebCore/bindings/v8/BindingState.cpp

    r126103 r126249  
    9999}
    100100
    101 void immediatelyReportUnsafeAccessTo(BindingState*, Document* targetDocument)
     101void printErrorMessageForFrame(Frame* frame, const String& message)
    102102{
    103     V8Proxy::reportUnsafeAccessTo(targetDocument);
     103    if (!frame)
     104        return;
     105    frame->document()->domWindow()->printErrorMessage(message);
    104106}
    105107

  • trunk/Source/WebCore/bindings/v8/BindingState.h

    r125592 r126249  
    3232#define BindingState_h
    3333
     34#include <wtf/text/WTFString.h>
     35
    3436namespace WebCore {
    3537
    … …  
    5658Document* currentDocument(BindingState*);
    5759
    58 void immediatelyReportUnsafeAccessTo(BindingState*, Document* targetDocument);
     60// FIXME: This function is redundant with the copy in JSDOMBinding.cpp.
     61void printErrorMessageForFrame(Frame*, const String& message);
    5962
    6063}

  • trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp

    r126002 r126249  
    154154{
    155155    Frame* target = getTargetFrame(host, data);
    156     if (target)
    157         V8Proxy::reportUnsafeAccessTo(target->document());
     156    if (!target)
     157        return;
     158    DOMWindow* targetWindow = target->document()->domWindow();
     159    targetWindow->printErrorMessage(targetWindow->crossDomainAccessErrorMessage(activeDOMWindow(BindingState::instance())));
    158160}
    159161

  • trunk/Source/WebCore/bindings/v8/V8Proxy.cpp

    r126222 r126249  
    7878namespace WebCore {
    7979
    80 void V8Proxy::reportUnsafeAccessTo(Document* targetDocument)
    81 {
    82     if (!targetDocument)
    83         return;
    84 
    85     // FIXME: We should pass both the active and target documents in as arguments.
    86     Frame* source = firstFrame(BindingState::instance());
    87     if (!source)
    88         return;
    89 
    90     Document* sourceDocument = source->document();
    91     if (!sourceDocument)
    92         return; // Ignore error if the source document is gone.
    93 
    94     // FIXME: This error message should contain more specifics of why the same
    95     // origin check has failed.
    96     String str = "Unsafe JavaScript attempt to access frame with URL " + targetDocument->url().string() +
    97                  " from frame with URL " + sourceDocument->url().string() + ". Domains, protocols and ports must match.\n";
    98 
    99     RefPtr<ScriptCallStack> stackTrace = createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true);
    100 
    101     // NOTE: Safari prints the message in the target page, but it seems like
    102     // it should be in the source page. Even for delayed messages, we put it in
    103     // the source page.
    104     sourceDocument->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, str, stackTrace.release());
    105 }
    106 
    10780// FIXME: This will be soon removed when we move runScript() to ScriptController.
    10881static v8::Local<v8::Value> handleMaxRecursionDepthExceeded()

  • trunk/Source/WebCore/bindings/v8/V8Proxy.h

    r126222 r126249  
    111111        V8DOMWindowShell* windowShell() const;
    112112
    113         static void reportUnsafeAccessTo(Document* targetDocument);
    114 
    115113        // FIXME: Move m_isolatedWorlds to ScriptController and remove this getter.
    116114        IsolatedWorldMap& isolatedWorlds() { return m_isolatedWorlds; }
Note: See TracChangeset for help on using the changeset viewer.