Changeset 100877 in webkit


Ignore:
Timestamp:
Nov 20, 2011 6:26:16 PM (12 years ago)
Author:
abarth@webkit.org
Message:

REGRESSION(r100691): Safari error pages and Growl notifications fail to load stylesheets
​https://bugs.webkit.org/show_bug.cgi?id=72836

Reviewed by Sam Weinig.

Source/WebCore:

This patch removes a (minor) security mitigation. Previously, we tried
sequester "directory listings" into unique origins to make it more
difficult for an attacker to crawl the user's local file system.
Unfortunately, this mitigation doesn't really buy us much security
because if the attacker has access to local files, we've probably lost
anyway.

The larger problem, however, is that this condition is overly
complicated and has broken in sublte ways several times in its
(relatively short) lifetime. In the cases reported in this bug, we see
that this check affects error pages in Safari and Growl notifications,
even those have nothing to do with directory listings.

If we have our heart set on this directory listing mitigation, we'll
need a more robust way of triggering the behavior than examining URLs
and guess whether they contain directory listings. For example, if we
implement Allow-From or Access-Control-Deny-Origin, then the embedder
can supply those policies along with the directory listings. Those
seem like much better solutions than the in-engine hack this patch
removes.

  • page/SecurityOrigin.cpp:

(WebCore::shouldTreatAsUniqueOrigin):

LayoutTests:

Update test results to show that XMLHttpRequets for directory listings
aren't blocked.

  • fast/xmlhttprequest/resources/xmlhttprequest-nonexistent-file-real.html:
  • fast/xmlhttprequest/xmlhttprequest-nonexistent-file-expected.txt:
Location:
trunk
Files:
1 deleted
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r100866 r100877  
     12011-11-20  Adam Barth  <abarth@webkit.org>
     2
     3        REGRESSION(r100691): Safari error pages and Growl notifications fail to load stylesheets
     4        https://bugs.webkit.org/show_bug.cgi?id=72836
     5
     6        Reviewed by Sam Weinig.
     7
     8        Update test results to show that XMLHttpRequets for directory listings
     9        aren't blocked.
     10
     11        * fast/xmlhttprequest/resources/xmlhttprequest-nonexistent-file-real.html:
     12        * fast/xmlhttprequest/xmlhttprequest-nonexistent-file-expected.txt:
     13
    1142011-11-19  James Robinson  <jamesr@chromium.org>
    215

  • trunk/LayoutTests/fast/xmlhttprequest/resources/xmlhttprequest-nonexistent-file-real.html

    r51294 r100877  
    2121        {
    2222            log("ReadyState handler: readyState = " + xhr.readyState);
     23
    2324            if (xhr.readyState == 4 && window.layoutTestController) {
     25                var results = window.top.document.getElementById('results');
     26                results.innerHTML = document.body.innerHTML;
     27
    2428                setTimeout("layoutTestController.notifyDone()", 0);
    2529            }

  • trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-nonexistent-file-expected.txt

    r62629 r100877  
    1 CONSOLE MESSAGE: line 1: XMLHttpRequest cannot load . Cross origin requests are only supported for HTTP.
    21
    32Bug 22475: REGRESSION: Async XMLHttpRequest never finishes on nonexistent files anymore
    … …  
    1312ReadyState handler: readyState = 1
    1413ReadyState handler: readyState = 4
    15 Error handler: readyState = 4
    1614

  • trunk/Source/WebCore/ChangeLog

    r100874 r100877  
     12011-11-20  Adam Barth  <abarth@webkit.org>
     2
     3        REGRESSION(r100691): Safari error pages and Growl notifications fail to load stylesheets
     4        https://bugs.webkit.org/show_bug.cgi?id=72836
     5
     6        Reviewed by Sam Weinig.
     7
     8        This patch removes a (minor) security mitigation.  Previously, we tried
     9        sequester "directory listings" into unique origins to make it more
     10        difficult for an attacker to crawl the user's local file system.
     11        Unfortunately, this mitigation doesn't really buy us much security
     12        because if the attacker has access to local files, we've probably lost
     13        anyway.
     14
     15        The larger problem, however, is that this condition is overly
     16        complicated and has broken in sublte ways several times in its
     17        (relatively short) lifetime.  In the cases reported in this bug, we see
     18        that this check affects error pages in Safari and Growl notifications,
     19        even those have nothing to do with directory listings.
     20
     21        If we have our heart set on this directory listing mitigation, we'll
     22        need a more robust way of triggering the behavior than examining URLs
     23        and guess whether they contain directory listings.  For example, if we
     24        implement Allow-From or Access-Control-Deny-Origin, then the embedder
     25        can supply those policies along with the directory listings.  Those
     26        seem like much better solutions than the in-engine hack this patch
     27        removes.
     28
     29        * page/SecurityOrigin.cpp:
     30        (WebCore::shouldTreatAsUniqueOrigin):
     31
    1322011-10-17  Antonio Gomes  <agomes@rim.com>
    233

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

    r100716 r100877  
    8787}
    8888
    89 static bool isDirectory(const String& path)
    90 {
    91     return path.endsWith("/");
    92 }
    93 
    9489static bool shouldTreatAsUniqueOrigin(const KURL& url)
    9590{
    … …  
    114109    if (SchemeRegistry::shouldTreatURLSchemeAsNoAccess(protocol))
    115110        return true;
    116 
    117     // We use unique origins for directory listings to make it harder to crawl
    118     // a local filesystem. Notice that we apply this protection only when we
    119     // use the outer URL for the security context because schemes that wrap
    120     // other URLs don't have directory listings.
    121     if (SchemeRegistry::shouldTreatURLSchemeAsLocal(protocol) && !shouldUseInnerURL(url)) {
    122         if (!innerURL.hasPath() || isDirectory(innerURL.path()))
    123             return true;
    124     }
    125111
    126112    // This is the common case.
Note: See TracChangeset for help on using the changeset viewer.