82495 - The view-source protocol handler is having troubles with security

Status: VERIFIED FIXED

(Core :: Security: CAPS, defect, P1)

Description

[rbs]

In:
===

NS_IMETHODIMP
nsScriptSecurityManager::CheckLoadURI()

the following assertion is firing when clicling on a view-source URL (see
example above):
===============

    NS_WARN_IF_FALSE(PR_FALSE, "unknown protocol in 
nsScriptSecurityManager::CheckLoadURI");


The full code of CheckLoadURI() is as follows:
==============================================

NS_IMETHODIMP
nsScriptSecurityManager::CheckLoadURI(nsIURI *aSourceURI, nsIURI *aTargetURI,
                                      PRUint32 aFlags)
{
    nsCOMPtr<nsIJARURI> jarURI;
    nsCOMPtr<nsIURI> sourceUri(aSourceURI);
    while((jarURI = do_QueryInterface(sourceUri)))
        jarURI->GetJARFile(getter_AddRefs(sourceUri));
    if (!sourceUri) return NS_ERROR_FAILURE;

    nsXPIDLCString sourceScheme;
    if (NS_FAILED(sourceUri->GetScheme(getter_Copies(sourceScheme))))
        return NS_ERROR_FAILURE;

    // Some loads are not allowed from mail/news messages
    if ((aFlags & nsIScriptSecurityManager::DISALLOW_FROM_MAIL) && 
        (nsCRT::strcasecmp(sourceScheme, "mailbox")  == 0 ||
         nsCRT::strcasecmp(sourceScheme, "imap")     == 0 ||
         nsCRT::strcasecmp(sourceScheme, "news")     == 0))
    {
        return NS_ERROR_DOM_BAD_URI;
    }

    nsCOMPtr<nsIURI> targetUri(aTargetURI);
    while((jarURI = do_QueryInterface(targetUri)))
        jarURI->GetJARFile(getter_AddRefs(targetUri));
    if (!targetUri) return NS_ERROR_FAILURE;

    nsXPIDLCString targetScheme;
    if (NS_FAILED(targetUri->GetScheme(getter_Copies(targetScheme))))
        return NS_ERROR_FAILURE;
    
    if (nsCRT::strcasecmp(targetScheme, sourceScheme) == 0)
    {
        // every scheme can access another URI from the same scheme
        return NS_OK;
    }

    enum Action { AllowProtocol, DenyProtocol, PrefControlled, ChromeProtocol, 
AboutProtocol };
    static const struct { 
        const char *name;
        Action action;
    } protocolList[] = {
        //-- Keep the most commonly used protocols at the top of the list
        //   to increase performance
        { "http",            AllowProtocol  },
        { "file",            PrefControlled },
        { "https",           AllowProtocol  },
        { "chrome",          ChromeProtocol },
        { "mailbox",         DenyProtocol   },
        { "pop",             AllowProtocol  },
        { "imap",            DenyProtocol   },
        { "pop3",            DenyProtocol   },
        { "news",            AllowProtocol  },
        { "javascript",      AllowProtocol  },
        { "ftp",             AllowProtocol  },
        { "about",           AboutProtocol  },
        { "mailto",          AllowProtocol  },
        { "aim",             AllowProtocol  },
        { "data",            AllowProtocol  },
        { "keyword",         DenyProtocol   },
        { "resource",        ChromeProtocol },
        { "gopher",          AllowProtocol  },
        { "datetime",        DenyProtocol   },
        { "finger",          AllowProtocol  },
        { "res",             DenyProtocol   }
    };

    nsXPIDLCString targetSpec;
    const char* targetPage;
    for (unsigned i=0; i < sizeof(protocolList)/sizeof(protocolList[0]); i++) {
        if (nsCRT::strcasecmp(targetScheme, protocolList[i].name) == 0) {
            PRBool doCheck = PR_FALSE;
            switch (protocolList[i].action) {
            case AllowProtocol:
                // everyone can access these schemes.
                return NS_OK;
            case PrefControlled:
                // Allow access if pref is false
                mPrefs->GetBoolPref("security.checkloaduri", &doCheck);
                return doCheck ? ReportErrorToConsole(aTargetURI) : NS_OK;
            case ChromeProtocol:
                if (aFlags & nsIScriptSecurityManager::ALLOW_CHROME)
                    return NS_OK;
                // resource: and chrome: are equivalent, securitywise
                if ((PL_strcmp(sourceScheme, "chrome") == 0) ||
                    (PL_strcmp(sourceScheme, "resource") == 0))
                    return NS_OK;
                return ReportErrorToConsole(aTargetURI);
            case AboutProtocol:
                // Allow loading about:blank, otherwise deny
                if(NS_FAILED(targetUri->GetSpec(getter_Copies(targetSpec))))
                    return NS_ERROR_FAILURE;
                targetPage = targetSpec.get() + sizeof("about:") - 1;
                return (PL_strcmp(targetPage, "blank") == 0)   || 
                       (PL_strcmp(targetPage, "") == 0)        ||
                       (PL_strcmp(targetPage, "mozilla") == 0) ||
                       (PL_strcmp(targetPage, "credits") == 0) ?
                    NS_OK : ReportErrorToConsole(aTargetURI);
            case DenyProtocol:
                // Deny access
                return ReportErrorToConsole(aTargetURI);
            }
        }
    }

    // If we reach here, we have an unknown protocol. Warn, but allow.
    // This is risky from a security standpoint, but allows flexibility
    // in installing new protocol handlers after initial ship.
    NS_WARN_IF_FALSE(PR_FALSE, "unknown protocol in 
nsScriptSecurityManager::CheckLoadURI");

    return NS_OK;
}

Comment 1

[Mitchell Stoltz (not reading bugmail)]

I will add view-source to the list of recognized protocols; thanks for catching
this. In the meantime, ignore the assertion; it does not indicate a lapse in
security.

Comment 2

[rbs]

Also, what do you think of the checking order below? I have bumped the rank of
'chrome' and 'resource' because in the mainstream cases (i.e, non-developer 
releases), most people will be browsing the web and reading e-mails and 
particularly interacting with the UI (changing menus/prefs...). Moreover,
since main stream folks don't usually edit html files for themselves, I pushed
'file' further down the check list. There could be futher tweaks if the
checking order is that much important performance-wise (for example, 'https'
is rarely used compared to some of the other protocols).

        { "http",            AllowProtocol  },
        { "chrome",          ChromeProtocol },
        { "resource",        ChromeProtocol },
        { "https",           AllowProtocol  },
        { "mailbox",         DenyProtocol   },
        { "pop",             AllowProtocol  },
        { "imap",            DenyProtocol   },
        { "pop3",            DenyProtocol   },
        { "news",            AllowProtocol  },
        { "javascript",      AllowProtocol  },
        { "file",            PrefControlled },
        { "ftp",             AllowProtocol  },
        { "about",           AboutProtocol  },
        { "mailto",          AllowProtocol  },
        { "aim",             AllowProtocol  },
        { "data",            AllowProtocol  },
        { "keyword",         DenyProtocol   },
        { "gopher",          AllowProtocol  },
        { "datetime",        DenyProtocol   },
        { "finger",          AllowProtocol  },
        { "res",             DenyProtocol   }

Comment 3

[Mitchell Stoltz (not reading bugmail)]

You're probably right about file:, but keep pop: and pop3: towards the bottom as
they're almost never used (possibly not at all, but they are in the source). The
real pop protocol is mailbox:. 

Comment 4

[rbs]

Note to myself:
When this is fixed, check if special-casing view-source could be removed in:

NS_IMETHODIMP
CSSLoaderImpl::LoadStyleLink(...)
  [...]
  //-- Make sure this page is allowed to load this URL
  // If we are doing view-source, no need to check...
  PRBool isForViewSource = PR_FALSE;
  if (aParserToUnblock) {
    nsAutoString command;
    aParserToUnblock->GetCommand(command);
    isForViewSource = command.Equals(NS_LITERAL_STRING("view-source"));
  }
  if (!isForViewSource) {
    nsresult rv;
    NS_WITH_SERVICE(nsIScriptSecurityManager, secMan, 
NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
    if (NS_FAILED(rv)) return rv;
    nsIURI* docURI;
    rv = mDocument->GetBaseURL(docURI);
    if (NS_FAILED(rv) || !docURI) return NS_ERROR_FAILURE;
    rv = secMan->CheckLoadURI(docURI, aURL, 
nsIScriptSecurityManager::ALLOW_CHROME);
    NS_IF_RELEASE(docURI);
    if (NS_FAILED(rv)) return rv;
  }
  [...]
}

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - add check for view-source in CHeckLoadURI and factor out some scheme checks into a separate function

Comment 6

[Jesse Ruderman]

r=jesse

Comment 7

[Mitchell Stoltz (not reading bugmail)]

Target is now 0.9.4, Priority P1.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.

Comment 9

[rbs]

Verified.