Changeset 29266 in webkit


Ignore:
Timestamp:
Jan 7, 2008, 5:30:27 PM (17 years ago)
Author:
weinig@apple.com
Message:

WebCore:

Reviewed by Sam Weinig

Fixes: ​http://bugs.webkit.org/show_bug.cgi?id=16523
<rdar://problem/5657447>

When a frame is created with the URL "about:blank" or "", it should
inherit its SecurityOrigin from its opener. However, once it has
decided on that SecurityOrigin, it should not change its mind.
Prior to this patch, several events could induce the frame to change
its SecurityOrigin, permitting an attacker to inject script into an
arbitrary SecurityOrigin.

This patch makes several changes:

1) Documents refuse to change from one SecurityOrigin to another

unless explicitly instructed to do so.

2) Navigating to a JavaScript URL that produces a value

preserves the current SecurityOrigin explicitly instead of
relying on the URL to preserve the origin (which fails for
about:blank URLs and SecurityOrigins with document.domain set).

Ideally, we should not preserve the URL at all. Instead, the
frame's URL should be the JavaScript URL, as in Firefox, but this
would require changes that are too risky for this patch. I'll
file this as a separate issue.

3) Various methods of navigating to JavaScript URLs were not

properly handling JavaScript that returned a value (and should
therefore replace the current document). This patch unifies
those code paths with the path that works.

There are still a handful of bugs relating to the handling of
JavaScript URLs, but I'll file those as separate issues.

Tests: http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html

http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
http/tests/security/aboutBlank/xss-DENIED-set-opener.html

  • dom/Document.cpp: (WebCore::Document::initSecurityOrigin):
  • dom/Document.h: (WebCore::Document::setSecurityOrigin):
  • loader/FrameLoader.cpp: (WebCore::FrameLoader::changeLocation): (WebCore::FrameLoader::urlSelected): (WebCore::FrameLoader::requestFrame): (WebCore::FrameLoader::submitForm): (WebCore::FrameLoader::executeIfJavaScriptURL): (WebCore::FrameLoader::begin):
  • loader/FrameLoader.h:
  • platform/SecurityOrigin.cpp: (WebCore::SecurityOrigin::setForURL): (WebCore::SecurityOrigin::createForFrame):
  • platform/SecurityOrigin.h:

LayoutTests:

Reviewed by Sam Weinig.

Fixes: ​http://bugs.webkit.org/show_bug.cgi?id=16523

Adds new LayoutTests for scripting from about:blank windows. These
windows should inherit its SecurityOrigin from its opener and should
refuse to change their origins when their opener changes exogenously
(the navigate-opener tests) or explicitly (the set-opener test).

  • http/tests/security/aboutBlank: Added.
  • http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write-expected.txt: Added.
  • http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html: Added.
  • http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url-expected.txt: Added.
  • http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html: Added.
  • http/tests/security/aboutBlank/xss-DENIED-set-opener-expected.txt: Added.
  • http/tests/security/aboutBlank/xss-DENIED-set-opener.html: Added.
  • http/tests/security/resources/innocent-victim-with-notify.html: Added.
  • http/tests/security/resources/innocent-victim.html: Added.
  • http/tests/security/resources/libwrapjs.js: Added.
  • http/tests/security/resources/open-window.html: Added.
Location:
trunk
Files:
11 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r29257 r29266  
     12008-01-07  Adam Barth  <hk9565@gmail.com>
     2
     3        Reviewed by Sam Weinig.
     4
     5        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16523
     6
     7        Adds new LayoutTests for scripting from about:blank windows.  These
     8        windows should inherit its SecurityOrigin from its opener and should
     9        refuse to change their origins when their opener changes exogenously
     10        (the navigate-opener tests) or explicitly (the set-opener test).
     11
     12        * http/tests/security/aboutBlank: Added.
     13        * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write-expected.txt: Added.
     14        * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html: Added.
     15        * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url-expected.txt: Added.
     16        * http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html: Added.
     17        * http/tests/security/aboutBlank/xss-DENIED-set-opener-expected.txt: Added.
     18        * http/tests/security/aboutBlank/xss-DENIED-set-opener.html: Added.
     19        * http/tests/security/resources/innocent-victim-with-notify.html: Added.
     20        * http/tests/security/resources/innocent-victim.html: Added.
     21        * http/tests/security/resources/libwrapjs.js: Added.
     22        * http/tests/security/resources/open-window.html: Added.
     23
    1242008-01-07  Adele Peterson  <adele@apple.com>
    225

  • trunk/LayoutTests/platform/win/Skipped

    r29254 r29266  
    197197http/tests/security/javascriptURL/xss-DENIED-from-javascript-url-in-foreign-domain-window-open.html
    198198http/tests/security/javascriptURL/xss-DENIED-to-javascript-url-in-foreign-domain-window-open.html
     199http/tests/security/aboutBlank/xss-DENIED-set-opener.html
     200http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html
     201http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
    199202
    200203# DRT is not fully implemented in boomer <rdar://problem/5128261>

  • trunk/WebCore/ChangeLog

    r29265 r29266  
     12008-01-07  Adam Barth  <hk9565@gmail.com>
     2
     3        Reviewed by Sam Weinig
     4
     5        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16523
     6        <rdar://problem/5657447>
     7
     8        When a frame is created with the URL "about:blank" or "", it should
     9        inherit its SecurityOrigin from its opener.  However, once it has
     10        decided on that SecurityOrigin, it should not change its mind.
     11        Prior to this patch, several events could induce the frame to change
     12        its SecurityOrigin, permitting an attacker to inject script into an
     13        arbitrary SecurityOrigin.
     14
     15        This patch makes several changes:
     16
     17        1) Documents refuse to change from one SecurityOrigin to another
     18           unless explicitly instructed to do so.
     19
     20        2) Navigating to a JavaScript URL that produces a value
     21           preserves the current SecurityOrigin explicitly instead of
     22           relying on the URL to preserve the origin (which fails for
     23           about:blank URLs and SecurityOrigins with document.domain set).
     24
     25           Ideally, we should not preserve the URL at all.  Instead, the
     26           frame's URL should be the JavaScript URL, as in Firefox, but this
     27           would require changes that are too risky for this patch.  I'll
     28           file this as a separate issue.
     29
     30        3) Various methods of navigating to JavaScript URLs were not
     31           properly handling JavaScript that returned a value (and should
     32           therefore replace the current document).  This patch unifies
     33           those code paths with the path that works.
     34
     35           There are still a handful of bugs relating to the handling of
     36           JavaScript URLs, but I'll file those as separate issues.
     37
     38        Tests: http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html
     39               http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
     40               http/tests/security/aboutBlank/xss-DENIED-set-opener.html
     41
     42        * dom/Document.cpp:
     43        (WebCore::Document::initSecurityOrigin):
     44        * dom/Document.h:
     45        (WebCore::Document::setSecurityOrigin):
     46        * loader/FrameLoader.cpp:
     47        (WebCore::FrameLoader::changeLocation):
     48        (WebCore::FrameLoader::urlSelected):
     49        (WebCore::FrameLoader::requestFrame):
     50        (WebCore::FrameLoader::submitForm):
     51        (WebCore::FrameLoader::executeIfJavaScriptURL):
     52        (WebCore::FrameLoader::begin):
     53        * loader/FrameLoader.h:
     54        * platform/SecurityOrigin.cpp:
     55        (WebCore::SecurityOrigin::setForURL):
     56        (WebCore::SecurityOrigin::createForFrame):
     57        * platform/SecurityOrigin.h:
     58
    1592008-01-07  Adele Peterson  <adele@apple.com>
    260

  • trunk/WebCore/dom/Document.cpp

    r29033 r29266  
    37173717void Document::initSecurityOrigin()
    37183718{
     3719    if (m_securityOrigin && !m_securityOrigin->isEmpty())
     3720        return;  // m_securityOrigin has already been initialized.
     3721
    37193722    m_securityOrigin = SecurityOrigin::createForFrame(m_frame);
    37203723}

  • trunk/WebCore/dom/Document.h

    r28912 r29266  
    850850    SecurityOrigin* securityOrigin() const { return m_securityOrigin.get(); }
    851851
     852    // Explicitly override the security origin for this document.
     853    // Note: It is dangerous to change the security origin of a document
     854    //       that already contains content.
     855    void setSecurityOrigin(SecurityOrigin* o) { m_securityOrigin = o; }
     856
    852857    bool processingLoadEvent() const { return m_processingLoadEvent; }
    853858

  • trunk/WebCore/loader/FrameLoader.cpp

    r29086 r29266  
    375375void FrameLoader::changeLocation(const KURL& url, const String& referrer, bool lockHistory, bool userGesture)
    376376{
    377     if (url.deprecatedString().find("javascript:", 0, false) == 0) {
    378         String script = KURL::decode_string(url.deprecatedString().mid(strlen("javascript:")));
    379         JSValue* result = executeScript(script, userGesture);
    380         String scriptResult;
    381         if (getString(result, scriptResult)) {
    382             begin(m_URL);
    383             write(scriptResult);
    384             end();
    385         }
    386         return;
    387     }
    388 
    389377    ResourceRequestCachePolicy policy = (m_cachePolicy == CachePolicyReload) || (m_cachePolicy == CachePolicyRefresh)
    390378        ? ReloadIgnoringCacheData : UseProtocolCachePolicy;
    … …  
    396384void FrameLoader::urlSelected(const ResourceRequest& request, const String& _target, Event* triggeringEvent, bool lockHistory, bool userGesture)
    397385{
     386    if (executeIfJavaScriptURL(request.url(), userGesture))
     387        return;
     388
    398389    String target = _target;
    399390    if (target.isEmpty() && m_frame->document())
    400391        target = m_frame->document()->baseTarget();
    401 
    402     const KURL& url = request.url();
    403     if (url.deprecatedString().startsWith("javascript:", false)) {
    404         executeScript(KURL::decode_string(url.deprecatedString().mid(strlen("javascript:"))), true);
    405         return;
    406     }
    407392
    408393    FrameLoadRequest frameRequest(request, target);
    … …  
    443428
    444429    if (!scriptURL.isEmpty())
    445         frame->loader()->replaceContentsWithScriptResult(scriptURL);
     430        frame->loader()->executeIfJavaScriptURL(scriptURL);
    446431
    447432    return true;
    … …  
    519504    if (urlString.startsWith("javascript:", false)) {
    520505        m_isExecutingJavaScriptFormAction = true;
    521         executeScript(KURL::decode_string(urlString.mid(strlen("javascript:"))));
     506        executeIfJavaScriptURL(u);
    522507        m_isExecutingJavaScriptFormAction = false;
    523508        return;
    … …  
    728713}
    729714
    730 void FrameLoader::replaceContentsWithScriptResult(const KURL& url)
    731 {
    732     JSValue* result = executeScript(KURL::decode_string(url.deprecatedString().mid(strlen("javascript:"))));
     715bool FrameLoader::executeIfJavaScriptURL(const KURL& url, bool userGesture)
     716{
     717    if (!url.deprecatedString().startsWith("javascript:", false))
     718        return false;
     719
     720    String script = KURL::decode_string(url.deprecatedString().mid(strlen("javascript:")));
     721    JSValue* result = executeScript(script, userGesture);
     722
    733723    String scriptResult;
    734724    if (!getString(result, scriptResult))
    735         return;
    736     begin();
     725        return true;
     726
     727    SecurityOrigin* currentSecurityOrigin = 0;
     728    if (m_frame->document())
     729        currentSecurityOrigin = m_frame->document()->securityOrigin();
     730
     731    begin(m_URL, true, currentSecurityOrigin);
    737732    write(scriptResult);
    738733    end();
     734
     735    return true;
    739736}
    740737
    … …  
    875872}
    876873
    877 void FrameLoader::begin(const KURL& url, bool dispatch)
    878 {
     874void FrameLoader::begin(const KURL& url, bool dispatch, SecurityOrigin* origin)
     875{
     876    // We need to take a reference to the security origin because |clear|
     877    // might destroy the document that owns it.
     878    RefPtr<SecurityOrigin> forcedSecurityOrigin = origin;
     879
    879880    bool resetScripting = !(m_isDisplayingInitialEmptyDocument && m_frame->document() && m_frame->document()->securityOrigin()->isSecureTransitionTo(url));
    880881    clear(resetScripting, resetScripting);
    … …  
    908909    if (m_decoder)
    909910        document->setDecoder(m_decoder.get());
     911    if (forcedSecurityOrigin)
     912        document->setSecurityOrigin(forcedSecurityOrigin.get());
    910913
    911914    updatePolicyBaseURL();

  • trunk/WebCore/loader/FrameLoader.h

    r28639 r29266  
    7676    class ResourceRequest;
    7777    class ResourceResponse;
     78    class SecurityOrigin;
    7879    class SharedBuffer;
    7980    class SubstituteData;
    … …  
    310311
    311312        void begin();
    312         void begin(const KURL&, bool dispatchWindowObjectAvailable = true);
     313        void begin(const KURL&, bool dispatchWindowObjectAvailable = true, SecurityOrigin* forcedSecurityOrigin = 0);
    313314
    314315        void write(const char* str, int len = -1, bool flush = false);
    … …  
    319320        void setEncoding(const String& encoding, bool userChosen);
    320321        String encoding() const;
     322
     323        // Returns true if url is a JavaScript URL.
     324        bool executeIfJavaScriptURL(const KURL& url, bool userGesture = false);
    321325
    322326        KJS::JSValue* executeScript(const String& url, int baseLine, const String& script);
    … …  
    477481        void setPolicyBaseURL(const String&);
    478482
    479         void replaceContentsWithScriptResult(const KURL&);
    480 
    481483        // Also not cool.
    482484        void stopLoadingSubframes();

  • trunk/WebCore/platform/SecurityOrigin.cpp

    r28912 r29266  
    4040namespace WebCore {
    4141
    42 SecurityOrigin::SecurityOrigin()
     42SecurityOrigin::SecurityOrigin(const KURL& url)
    4343    : m_port(0)
    4444    , m_portSet(false)
    … …  
    4646    , m_domainWasSetInDOM(false)
    4747{
    48 }
     48    if (url.isEmpty())
     49      return;
    4950
    50 void SecurityOrigin::clear()
    51 {
    52     m_protocol = String();
    53     m_host = String();
    54     m_port = 0;
    55     m_portSet = false;
    56     m_noAccess = false;
    57     m_domainWasSetInDOM = false;
     51    m_protocol = url.protocol().lower();
     52
     53    // These protocols do not represent principals.
     54    if (m_protocol == "about" || m_protocol == "javascript")
     55        m_protocol = String();
     56
     57    if (m_protocol.isEmpty())
     58        return;
     59
     60    // data: URLs are not allowed access to anything other than themselves.
     61    if (m_protocol == "data")
     62        m_noAccess = true;
     63
     64    m_host = url.host().lower();
     65    m_port = url.port();
     66
     67    if (m_port)
     68        m_portSet = true;
    5869}
    5970
    … …  
    6374}
    6475
    65 void SecurityOrigin::setForURL(const KURL& url)
    66 {
    67     clear();
    68 
    69     if (url.isEmpty())
    70       return;
    71 
    72     m_protocol = url.protocol().lower();
    73     m_host = url.host().lower();
    74     m_port = url.port();
    75 
    76     if (m_port)
    77         m_portSet = true;
    78 
    79     // data: URLs are not allowed access to anything other than themselves.
    80     if (m_protocol == "data")
    81         m_noAccess = true;
    82 }
    83 
    8476PassRefPtr<SecurityOrigin> SecurityOrigin::createForFrame(Frame* frame)
    8577{
    86     RefPtr<SecurityOrigin> origin = new SecurityOrigin();
     78    if (!frame)
     79        return new SecurityOrigin(KURL());
    8780
    88     if (!frame)
     81    FrameLoader* loader = frame->loader();
     82
     83    RefPtr<SecurityOrigin> origin = new SecurityOrigin(loader->url());
     84    if (!origin->isEmpty())
    8985        return origin;
    9086
    91     FrameLoader* loader = frame->loader();
    92     const KURL& securityPolicyURL = loader->url();
    93 
    94     origin->setForURL(securityPolicyURL);
    95 
    96     if (!origin->isEmpty() && origin->m_protocol != "about")
    97         return origin;
    98 
    99     // In the case of about:blank or javascript: URLs (which create
    100     // documents using the "about" protocol) do we want to use the
    101     // parent or openers origin.
     87    // If we do not obtain a principal from the URL, then we try to find a
     88    // principal via the frame hierarchy.
    10289
    10390    Frame* openerFrame = frame->tree()->parent();

  • trunk/WebCore/platform/SecurityOrigin.h

    r28912 r29266  
    5151        bool isSecureTransitionTo(const KURL&) const;
    5252
     53        bool isEmpty() const;
    5354        String toString() const;
    5455       
    … …  
    5657       
    5758    private:
    58         SecurityOrigin();
    59         bool isEmpty() const;
    60 
    61         void clear();
    62         void setForURL(const KURL& url);
     59        SecurityOrigin(const KURL& url);
    6360
    6461        String m_protocol;
Note: See TracChangeset for help on using the changeset viewer.