Changeset 218835 in webkit


Ignore:
Timestamp:
Jun 27, 2017 8:59:48 AM (7 years ago)
Author:
fred.wang@free.fr
Message:

Some tests to verify forbidden frame navigation time out
​https://bugs.webkit.org/show_bug.cgi?id=173657

Patch by Frederic Wang <​fwang@igalia.com> on 2017-06-27
Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

  • web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation-2-expected.txt: Update the text expectation to PASS.
  • web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation_without_user_gesture-expected.txt: Ditto.
  • web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_ancestor-1-expected.txt: Ditto.
  • web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3-expected.txt: Add the security error until bug 173162 is fixed.
  • web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3-expected.txt: Ditto.

Source/WebCore:

Currently some tests try and perform a forbidden frame navigation and verify the
corresponding console error. However, WebKit does not raise any exception for such error so
the tests have to wait until the timeout limit to complete, which makes execution slow.
This patch modifies the setters of window.location for which such error may happen in order
to raise an exception so the tests behave as expected.

No new tests, already covered by existing tests.

  • page/Location.cpp: Adjust Location::setLocation to return a security exception and pass it

to the callers.
(WebCore::Location::setHref): Adjust function to possibly return an exception.
(WebCore::Location::setProtocol): Ditto.
(WebCore::Location::setHost): Ditto.
(WebCore::Location::setHostname): Ditto.
(WebCore::Location::setPort): Ditto.
(WebCore::Location::setPathname): Ditto.
(WebCore::Location::setSearch): Ditto.
(WebCore::Location::setHash): Ditto.
(WebCore::Location::assign): Ditto.
(WebCore::Location::setLocation): FrameLoader::findFrameForNavigation is really only used
to verify whether navigating m_frame is permitted so it is more simple and clearer to do it
directly. When navigation is not permitted, this function now raises a security exception.

  • page/Location.h: Modify some setters to return an ExceptionOr<void>.
  • page/Location.idl: Allow some setters to raise an exception.

LayoutTests:

  • fast/frames/sandboxed-iframe-navigation-top-denied-expected.txt: Add the security error.
  • http/tests/security/frameNavigation/inactive-function-in-popup-navigate-child.html: Adjust

the test to catch and dump the exception and complete immediately.

  • http/tests/security/frameNavigation/inactive-function-in-popup-navigate-child-expected.txt:

Add the dumped security error exception.

Location:
trunk
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r218833 r218835  
     12017-06-27  Frederic Wang  <fwang@igalia.com>
     2
     3        Some tests to verify forbidden frame navigation time out
     4        https://bugs.webkit.org/show_bug.cgi?id=173657
     5
     6        Reviewed by Chris Dumez.
     7
     8        * fast/frames/sandboxed-iframe-navigation-top-denied-expected.txt: Add the security error.
     9        * http/tests/security/frameNavigation/inactive-function-in-popup-navigate-child.html: Adjust
     10        the test to catch and dump the exception and complete immediately.
     11        * http/tests/security/frameNavigation/inactive-function-in-popup-navigate-child-expected.txt:
     12        Add the dumped security error exception.
     13
    1142017-06-27  Youenn Fablet  <youenn@apple.com>
    215

  • trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-top-denied-expected.txt

    r138517 r218835  
    11CONSOLE MESSAGE: Unsafe JavaScript attempt to initiate navigation for frame with URL 'navigate-top-to-fail.html'. The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set.
    22
     3CONSOLE MESSAGE: SecurityError (DOM Exception 18): The operation is insecure.
    34This test verifies that a sandboxed IFrame cannot navigate the top-level frame without allow-top-navigation. This test passes if the navigation does not occur.
    45

  • trunk/LayoutTests/http/tests/security/frameNavigation/inactive-function-in-popup-navigate-child-expected.txt

    r112184 r218835  
    33iframe-with-inner-frame-on-foreign-domain-LOADED
    44Attempting navigation...
     5SecurityError (DOM Exception 18): The operation is insecure.

  • trunk/LayoutTests/http/tests/security/frameNavigation/inactive-function-in-popup-navigate-child.html

    r120174 r218835  
    3131    if (e.data = "iframe-with-inner-frame-on-foreign-domain-LOADED") {
    3232        log("Attempting navigation...");
    33         window.savedFunction();
    34         setTimeout(function() {
    35             // Unfortunately, there's no way to receive positive confirmation
    36             // that the navigation failed, so we just complete the test
    37             // asynchronously.
     33        try {
     34            window.savedFunction();
    3835            if (window.testRunner)
    3936                testRunner.notifyDone();
    40         }, 0);
     37        } catch(e) {
     38            log(e);
     39            testRunner.notifyDone();
     40        }
    4141        return;
    4242    }

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r218773 r218835  
     12017-06-27  Frederic Wang  <fwang@igalia.com>
     2
     3        Some tests to verify forbidden frame navigation time out
     4        https://bugs.webkit.org/show_bug.cgi?id=173657
     5
     6        Reviewed by Chris Dumez.
     7
     8        * web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation-2-expected.txt: Update the text expectation to PASS.
     9        * web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation_without_user_gesture-expected.txt: Ditto.
     10        * web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_ancestor-1-expected.txt: Ditto.
     11        * web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3-expected.txt: Add the security error until bug 173162 is fixed.
     12        * web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3-expected.txt: Ditto.
     13
    1142017-06-23  Youenn Fablet  <youenn@apple.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation-2-expected.txt

    r218639 r218835  
    33
    44
    5 Harness Error (TIMEOUT), message = null
     5PASS Frames without `allow-top-navigation` should not be able to navigate the top frame.
    66
    7 TIMEOUT Frames without `allow-top-navigation` should not be able to navigate the top frame. Test timed out
    8 

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_allow_top_navigation_by_user_activation_without_user_gesture-expected.txt

    r213882 r218835  
    66
    77
    8 FAIL The sandboxed iframe should post a message saying the test was in the state of 'PASS'. assert_equals: The message should say 'PASS' instead of 'FAIL' expected "PASS" but got "FAIL"
     8PASS The sandboxed iframe should post a message saying the test was in the state of 'PASS'.
    99

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_ancestor-1-expected.txt

    r218639 r218835  
    33
    44
    5 Harness Error (TIMEOUT), message = null
     5PASS Check that sandboxed iframe can not navigate their ancestors
    66
    7 NOTRUN Check that sandboxed iframe can not navigate their ancestors
    8 

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3-expected.txt

    r218000 r218835  
    11CONSOLE MESSAGE: line 15: Unsafe JavaScript attempt to initiate navigation for frame with URL 'about:blank' from frame with URL 'http://localhost:8800/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_helper-3.html'. The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors.
    22
     3CONSOLE MESSAGE: line 15: SecurityError (DOM Exception 18): The operation is insecure.
    34
    45

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3-expected.txt

    r206999 r218835  
    11CONSOLE MESSAGE: line 15: Unsafe JavaScript attempt to initiate navigation for frame with URL 'about:blank' from frame with URL 'http://localhost:8800/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_helper-3.html'. The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors.
    22
     3CONSOLE MESSAGE: line 15: SecurityError (DOM Exception 18): The operation is insecure.
    34
    45

  • trunk/Source/WebCore/ChangeLog

    r218832 r218835  
     12017-06-27  Frederic Wang  <fwang@igalia.com>
     2
     3        Some tests to verify forbidden frame navigation time out
     4        https://bugs.webkit.org/show_bug.cgi?id=173657
     5
     6        Reviewed by Chris Dumez.
     7
     8        Currently some tests try and perform a forbidden frame navigation and verify the
     9        corresponding console error. However, WebKit does not raise any exception for such error so
     10        the tests have to wait until the timeout limit to complete, which makes execution slow.
     11        This patch modifies the setters of window.location for which such error may happen in order
     12        to raise an exception so the tests behave as expected.
     13
     14        No new tests, already covered by existing tests.
     15
     16        * page/Location.cpp: Adjust Location::setLocation to return a security exception and pass it
     17        to the callers.
     18        (WebCore::Location::setHref): Adjust function to possibly return an exception.
     19        (WebCore::Location::setProtocol): Ditto.
     20        (WebCore::Location::setHost): Ditto.
     21        (WebCore::Location::setHostname): Ditto.
     22        (WebCore::Location::setPort): Ditto.
     23        (WebCore::Location::setPathname): Ditto.
     24        (WebCore::Location::setSearch): Ditto.
     25        (WebCore::Location::setHash): Ditto.
     26        (WebCore::Location::assign): Ditto.
     27        (WebCore::Location::setLocation): FrameLoader::findFrameForNavigation is really only used
     28        to verify whether navigating m_frame is permitted so it is more simple and clearer to do it
     29        directly. When navigation is not permitted, this function now raises a security exception.
     30        * page/Location.h: Modify some setters to return an ExceptionOr<void>.
     31        * page/Location.idl: Allow some setters to raise an exception.
     32
    1332017-06-26  Fujii Hironori  <Hironori.Fujii@sony.com>
    234

  • trunk/Source/WebCore/page/Location.cpp

    r210859 r218835  
    151151}
    152152
    153 void Location::setHref(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
    154 {
    155     if (!m_frame)
    156         return;
    157     setLocation(activeWindow, firstWindow, url);
     153ExceptionOr<void> Location::setHref(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
     154{
     155    if (!m_frame)
     156        return { };
     157    return setLocation(activeWindow, firstWindow, url);
    158158}
    159159
    … …  
    165165    if (!url.setProtocol(protocol))
    166166        return Exception { SYNTAX_ERR };
    167     setLocation(activeWindow, firstWindow, url.string());
    168     return { };
    169 }
    170 
    171 void Location::setHost(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& host)
    172 {
    173     if (!m_frame)
    174         return;
     167    return setLocation(activeWindow, firstWindow, url.string());
     168}
     169
     170ExceptionOr<void> Location::setHost(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& host)
     171{
     172    if (!m_frame)
     173        return { };
    175174    URL url = m_frame->document()->url();
    176175    url.setHostAndPort(host);
    177     setLocation(activeWindow, firstWindow, url.string());
    178 }
    179 
    180 void Location::setHostname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& hostname)
    181 {
    182     if (!m_frame)
    183         return;
     176    return setLocation(activeWindow, firstWindow, url.string());
     177}
     178
     179ExceptionOr<void> Location::setHostname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& hostname)
     180{
     181    if (!m_frame)
     182        return { };
    184183    URL url = m_frame->document()->url();
    185184    url.setHost(hostname);
    186     setLocation(activeWindow, firstWindow, url.string());
    187 }
    188 
    189 void Location::setPort(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& portString)
    190 {
    191     if (!m_frame)
    192         return;
     185    return setLocation(activeWindow, firstWindow, url.string());
     186}
     187
     188ExceptionOr<void> Location::setPort(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& portString)
     189{
     190    if (!m_frame)
     191        return { };
    193192    URL url = m_frame->document()->url();
    194193    int port = portString.toInt();
    … …  
    197196    else
    198197        url.setPort(port);
    199     setLocation(activeWindow, firstWindow, url.string());
    200 }
    201 
    202 void Location::setPathname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& pathname)
    203 {
    204     if (!m_frame)
    205         return;
     198    return setLocation(activeWindow, firstWindow, url.string());
     199}
     200
     201ExceptionOr<void> Location::setPathname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& pathname)
     202{
     203    if (!m_frame)
     204        return { };
    206205    URL url = m_frame->document()->url();
    207206    url.setPath(pathname);
    208     setLocation(activeWindow, firstWindow, url.string());
    209 }
    210 
    211 void Location::setSearch(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& search)
    212 {
    213     if (!m_frame)
    214         return;
     207    return setLocation(activeWindow, firstWindow, url.string());
     208}
     209
     210ExceptionOr<void> Location::setSearch(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& search)
     211{
     212    if (!m_frame)
     213        return { };
    215214    URL url = m_frame->document()->url();
    216215    url.setQuery(search);
    217     setLocation(activeWindow, firstWindow, url.string());
    218 }
    219 
    220 void Location::setHash(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& hash)
    221 {
    222     if (!m_frame)
    223         return;
     216    return setLocation(activeWindow, firstWindow, url.string());
     217}
     218
     219ExceptionOr<void> Location::setHash(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& hash)
     220{
     221    if (!m_frame)
     222        return { };
    224223    ASSERT(m_frame->document());
    225224    auto url = m_frame->document()->url();
    … …  
    233232    // cases where fragment identifiers are ignored or invalid.
    234233    if (equalIgnoringNullity(oldFragmentIdentifier, url.fragmentIdentifier()))
    235         return;
    236     setLocation(activeWindow, firstWindow, url.string());
    237 }
    238 
    239 void Location::assign(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
    240 {
    241     if (!m_frame)
    242         return;
    243     setLocation(activeWindow, firstWindow, url);
     234        return { };
     235    return setLocation(activeWindow, firstWindow, url.string());
     236}
     237
     238ExceptionOr<void> Location::assign(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
     239{
     240    if (!m_frame)
     241        return { };
     242    return setLocation(activeWindow, firstWindow, url);
    244243}
    245244
    … …  
    281280}
    282281
    283 void Location::setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
     282ExceptionOr<void> Location::setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
    284283{
    285284    ASSERT(m_frame);
    286     auto* targetFrame = m_frame->loader().findFrameForNavigation({ }, activeWindow.document());
    287     if (!targetFrame)
    288         return;
    289     ASSERT(targetFrame->document());
    290     ASSERT(targetFrame->document()->domWindow());
    291     targetFrame->document()->domWindow()->setLocation(activeWindow, firstWindow, url);
     285    if (!activeWindow.document()->canNavigate(m_frame))
     286        return Exception { SECURITY_ERR };
     287    ASSERT(m_frame->document());
     288    ASSERT(m_frame->document()->domWindow());
     289    m_frame->document()->domWindow()->setLocation(activeWindow, firstWindow, url);
     290    return { };
    292291}
    293292

  • trunk/Source/WebCore/page/Location.h

    r209841 r218835  
    4444    static Ref<Location> create(Frame* frame) { return adoptRef(*new Location(frame)); }
    4545
    46     void setHref(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     46    ExceptionOr<void> setHref(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    4747    String href() const;
    4848
    49     void assign(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     49    ExceptionOr<void> assign(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    5050    void replace(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    5151    void reload(DOMWindow& activeWindow);
    … …  
    5353    ExceptionOr<void> setProtocol(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    5454    String protocol() const;
    55     void setHost(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     55    ExceptionOr<void> setHost(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    5656    String host() const;
    57     void setHostname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     57    ExceptionOr<void> setHostname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    5858    String hostname() const;
    59     void setPort(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     59    ExceptionOr<void> setPort(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    6060    String port() const;
    61     void setPathname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     61    ExceptionOr<void> setPathname(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    6262    String pathname() const;
    63     void setSearch(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     63    ExceptionOr<void> setSearch(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    6464    String search() const;
    65     void setHash(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     65    ExceptionOr<void> setHash(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    6666    String hash() const;
    6767    String origin() const;
    … …  
    7474    explicit Location(Frame*);
    7575
    76     void setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
     76    ExceptionOr<void> setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
    7777
    7878    const URL& url() const;

  • trunk/Source/WebCore/page/Location.idl

    r217773 r218835  
    4343    Unforgeable,
    4444] interface Location {
    45     [SetterCallWith=ActiveWindow&FirstWindow, DoNotCheckSecurityOnSetter] stringifier attribute USVString href;
     45    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException, DoNotCheckSecurityOnSetter] stringifier attribute USVString href;
    4646
    47     [CallWith=ActiveWindow&FirstWindow, ForwardDeclareInHeader] void assign(USVString url);
     47    [CallWith=ActiveWindow&FirstWindow, MayThrowException, ForwardDeclareInHeader] void assign(USVString url);
    4848    [DoNotCheckSecurity, CallWith=ActiveWindow&FirstWindow, ForwardDeclareInHeader] void replace(USVString url);
    4949    [CallWith=ActiveWindow, ForwardDeclareInHeader] void reload();
    … …  
    5151    // URI decomposition attributes
    5252    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString protocol;
    53     [SetterCallWith=ActiveWindow&FirstWindow] attribute USVString host;
    54     [SetterCallWith=ActiveWindow&FirstWindow] attribute USVString hostname;
    55     [SetterCallWith=ActiveWindow&FirstWindow] attribute USVString port;
    56     [SetterCallWith=ActiveWindow&FirstWindow] attribute USVString pathname;
    57     [SetterCallWith=ActiveWindow&FirstWindow] attribute USVString search;
    58     [SetterCallWith=ActiveWindow&FirstWindow] attribute USVString hash;
     53    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString host;
     54    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString hostname;
     55    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString port;
     56    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString pathname;
     57    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString search;
     58    [SetterCallWith=ActiveWindow&FirstWindow, SetterMayThrowException] attribute USVString hash;
    5959
    6060    readonly attribute USVString origin;
Note: See TracChangeset for help on using the changeset viewer.