Skip to content

Commit

Permalink
Cherry-pick 265870.131@safari-7616-branch (d2e3954). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=259235

    Check if external entity loads from libxslt are allowed before loading them
    https://bugs.webkit.org/show_bug.cgi?id=259235
    rdar://111457167

    Reviewed by David Kilzer.

    Otherwise tricky use of libxslt can make arbitrary file loads to files allowed by the
    web content process's sandbox.  We should limit it to what the current security origin
    can request.

    Monterey has an older version of libxml2 which fails differently in this case.
    Tests exist that verify that allowed external entities are still allowed.
    The important thing is that the contents of the files are not in the Monterey test expectations.

    * LayoutTests/http/tests/security/resources/xslt-external-entity.svg: Added.
    * LayoutTests/http/tests/security/resources/xslt2.py: Added.
    * LayoutTests/http/tests/security/xslt-external-entity-expected.txt: Added.
    * LayoutTests/http/tests/security/xslt-external-entity.html: Added.
    * Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:
    (WebCore::shouldAllowExternalLoad):
    (WebCore::entityLoader):
    (WebCore::initializeXMLParser):

    Canonical link: https://commits.webkit.org/265870.131@safari-7616-branch
  • Loading branch information
achristensen07 authored and mcatanzaro committed Sep 26, 2023
1 parent 0382703 commit 731e3fb
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 2 deletions.
15 changes: 15 additions & 0 deletions LayoutTests/http/tests/security/resources/xslt-external-entity.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
26 changes: 26 additions & 0 deletions LayoutTests/http/tests/security/resources/xslt2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env python3

import base64
import os
import sys

sys.stdout.write('Access-Control-Allow-Origin: *\r\n')
sys.stdout.write('\r\n')
sys.stdout.write('<!DOCTYPE p [')
sys.stdout.write('<!ENTITY passwd SYSTEM "file:///etc/passwd">')
sys.stdout.write('<!ENTITY hosts SYSTEM "file:///etc/hosts">')
sys.stdout.write('<!ENTITY group SYSTEM "file://localhost/etc/group">')
sys.stdout.write(']>')
sys.stdout.write('<p>')
sys.stdout.write(' <p style="border-style: dotted;" id="pw">/etc/passwd:')
sys.stdout.write('&passwd;')
sys.stdout.write(' </p>')
sys.stdout.write(' <p style="border-style: dotted;">/etc/hosts:')
sys.stdout.write('&hosts;')
sys.stdout.write(' </p>')
sys.stdout.write(' <p style="border-style: dotted;">/etc/group:')
sys.stdout.write('&group;')
sys.stdout.write(' </p>')
sys.stdout.write('</p>')
sys.stdout.write('')
sys.exit(0)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALERT: document() /etc/passwd: /etc/hosts: /etc/group:

12 changes: 12 additions & 0 deletions LayoutTests/http/tests/security/xslt-external-entity.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
if (window.testRunner) { testRunner.dumpAsText() }
function go() {
const i = document.getElementById('i');
const doc = i.contentWindow.document;
const div = doc.getElementsByTagName("div")[0];
const text = div.innerText;
alert(text);
}
</script>

<iframe id=i src="resources/xslt-external-entity.svg" width=500 height=500 onload=go()></iframe>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
layer at (0,0) size 800x600
RenderView at (0,0) size 800x600
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
CONSOLE MESSAGE: Failure to process entity passwd

CONSOLE MESSAGE: Entity 'passwd' not defined

CONSOLE MESSAGE: Failure to process entity hosts

CONSOLE MESSAGE: Entity 'hosts' not defined

CONSOLE MESSAGE: Failure to process entity group

CONSOLE MESSAGE: Entity 'group' not defined

ALERT: document()

Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
CONSOLE MESSAGE: Unsafe attempt to load URL http://localhost:8000/security/resources/target.xml from origin http://127.0.0.1:8000. Domains, protocols and ports must match.

CONSOLE MESSAGE: Unsafe attempt to load URL http://localhost:8000/security/resources/target.xml from origin http://127.0.0.1:8000. Domains, protocols and ports must match.
CONSOLE MESSAGE: Failure to process entity ent

This test includes a cross-origin external entity. It passes if the load fails and thus there is no text below this line.
CONSOLE MESSAGE: Entity 'ent' not defined

layer at (0,0) size 800x600
RenderView at (0,0) size 800x600
13 changes: 13 additions & 0 deletions Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ static bool shouldAllowExternalLoad(const URL& url)
// retrieved content. If we had more context, we could potentially allow
// the parser to load a DTD. As things stand, we take the conservative
// route and allow same-origin requests only.
if (!XMLDocumentParserScope::currentCachedResourceLoader || !XMLDocumentParserScope::currentCachedResourceLoader->document())
return false;
if (!XMLDocumentParserScope::currentCachedResourceLoader->document()->securityOrigin().canRequest(url, OriginAccessPatternsForWebProcess::singleton())) {
XMLDocumentParserScope::currentCachedResourceLoader->printAccessDeniedMessage(url);
return false;
Expand Down Expand Up @@ -535,13 +537,24 @@ static void errorFunc(void*, const char*, ...)
}
#endif

static xmlExternalEntityLoader defaultEntityLoader { nullptr };

static xmlParserInputPtr entityLoader(const char* url, const char* id, xmlParserCtxtPtr context)
{
if (!shouldAllowExternalLoad(URL(String::fromUTF8(url))))
return nullptr;
return defaultEntityLoader(url, id, context);
}

static void initializeXMLParser()
{
static std::once_flag flag;
std::call_once(flag, [&] {
xmlInitParser();
xmlRegisterInputCallbacks(matchFunc, openFunc, readFunc, closeFunc);
xmlRegisterOutputCallbacks(matchFunc, openFunc, writeFunc, closeFunc);
defaultEntityLoader = xmlGetExternalEntityLoader();
xmlSetExternalEntityLoader(entityLoader);
libxmlLoaderThread = &Thread::current();
});
}
Expand Down

0 comments on commit 731e3fb

Please sign in to comment.