Skip to content

GitLab

T
tangled
Commit 94a6ff53 authored by mkwst's avatar mkwst Committed by Commit bot
Browse files
Options

CSP: Strip reported URLs for 'frame-src' and 'object-src'. 

The relaxation that landed in https://codereview.chromium.org/2002943002
was a bit too relaxed, and leaks navigation targets cross-origin for
'frame-src' and 'object-src' violations.

This patch reverts to the old behavior for those two directives.

BUG=633306

Review-Url: https://codereview.chromium.org/2255103002
Cr-Commit-Position: refs/heads/master@{#412809}
parent 21f7b3ae
Hide whitespace changes
Inline Side-by-side
Showing with 92 additions and 41 deletions
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-blocked-expected.txt deleted 100644 → 0
View file @ 21f7b3ae
CONSOLE ERROR: line 3: Refused to frame 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/alert-fail.html' because it violates the following Content Security Policy directive: "frame-src 'none'".
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-blocked.html
View file @ 94a6ff53
<meta http-equiv="Content-Security-Policy" content="frame-src 'none'">
<script src="resources/dump-as-text.js"></script>
<iframe src="resources/alert-fail.html"></iframe>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<meta http-equiv="Content-Security-Policy" content="frame-src 'self'">
<script>
async_test(t => {
var watcher = new EventWatcher(t, document, ['securitypolicyviolation']);
watcher
.wait_for('securitypolicyviolation')
.then(t.step_func(e => {
assert_equals(e.blockedURI, "http://localhost:8000", "The reported URL should be stripped.");
assert_equals(e.lineNumber, 21, "IFrame injected from script on this page.");
t.done();
}));
window.onmessage = t.unreached_func('No message should be sent from the frame.');
window.onload = _ => {
var url = "http://localhost:8000/security/resources/post-done.html";
var i = document.createElement('iframe');
i.src = url;
document.body.appendChild(i);
};
}, "The unredirected frame is blocked.");
</script>
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin-expected.txt deleted 100644 → 0
View file @ 21f7b3ae
CONSOLE ERROR: line 7: Refused to frame 'http://localhost:8000/security/contentSecurityPolicy/resources/target-for-frame-that-navigates-itself.html' because it violates the following Content Security Policy directive: "frame-src https://localhost:8443".
The test verifies that Content-Security-Policy from the main frame restricts child frame's location even when the location is changed as a result of a navigation trigerred from within the child frame (which might reside in another renderer process due to --site-per-process).
--------
Frame: '<!--framePath //<!--frame0-->-->'
--------
Frame that navigates itself to an origin other than https://localhost:8443. Automatically clicked link.
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin.html
View file @ 94a6ff53
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Security-Policy" content="frame-src https://localhost:8443">
<script>
testRunner.dumpAsText();
testRunner.dumpChildFramesAsText();
</script>
</head>
<body>
The test verifies that Content-Security-Policy from the main frame
restricts child frame's location even when the location is changed
as a result of a navigation trigerred from within the child frame
(which might reside in another renderer process due to --site-per-process).
<iframe src="https://localhost:8443/security/contentSecurityPolicy/resources/frame-that-navigates-itself.html"></iframe>
</body>
</html>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<meta http-equiv="Content-Security-Policy" content="frame-src https://localhost:8443">
<script>
async_test(t => {
var watcher = new EventWatcher(t, document, ['securitypolicyviolation']);
watcher
.wait_for('securitypolicyviolation')
.then(t.step_func(e => {
assert_equals(e.blockedURI, "http://localhost:8000", "The reported URL should be stripped.");
assert_equals(e.lineNumber, 7, "This shouldn't actually be 7, since it happens in a cross-origin frame. :/");
t.done();
}));
window.onmessage = t.unreached_func('No message should be sent from the frame.');
window.onload = _ => {
// The test verifies that Content-Security-Policy from the main frame
// restricts child frame's location even when the location is changed
// as a result of a navigation trigerred from within the child frame
// (which might reside in another renderer process due to --site-per-process).
var url = "https://localhost:8443/security/contentSecurityPolicy/resources/frame-that-navigates-itself.html";
var i = document.createElement('iframe');
i.src = url;
document.body.appendChild(i);
};
}, "The frame's navigation is blocked.");
</script>
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked-expected.txt deleted 100644 → 0
View file @ 21f7b3ae
CONSOLE ERROR: Refused to frame 'http://localhost:8000/security/contentSecurityPolicy/resources/alert-fail.html' because it violates the following Content Security Policy directive: "frame-src 127.0.0.1:8000".
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked.html
View file @ 94a6ff53
<meta http-equiv="Content-Security-Policy" content="frame-src 127.0.0.1:8000">
<script src="resources/dump-as-text.js"></script>
<iframe src="resources/redir.php?url=http://localhost:8000/security/contentSecurityPolicy/resources/alert-fail.html"></iframe>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<meta http-equiv="Content-Security-Policy" content="frame-src 'self'">
<script>
async_test(t => {
var watcher = new EventWatcher(t, document, ['securitypolicyviolation']);
watcher
.wait_for('securitypolicyviolation')
.then(t.step_func(e => {
assert_equals(e.blockedURI, "http://localhost:8000", "The reported URL should be stripped.");
assert_equals(e.lineNumber, 0, "No script involved in the redirect.");
t.done();
}));
window.onmessage = t.unreached_func('No message should be sent from the frame.');
window.onload = _ => {
var url = "resources/redir.php?url=http://localhost:8000/security/resources/post-done.html";
var i = document.createElement('iframe');
i.src = url;
document.body.appendChild(i);
};
}, "The redirected frame is blocked.");
</script>
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/target-for-frame-that-navigates-itself.html
View file @ 94a6ff53
......@@ -3,5 +3,8 @@
<body>
We should not get here - CSP from frame-src-child-frame-navigates-itself.html
should prevent navigation to an origin other than https://localhost:8443.
<script>
top.postMessage('loaded', '*');
</script>
</body>
</html>
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
View file @ 94a6ff53
......@@ -779,13 +779,22 @@ void ContentSecurityPolicy::upgradeInsecureRequests()
m_insecureRequestPolicy |= kUpgradeInsecureRequests;
}
static String stripURLForUseInReport(Document* document, const KURL& url, RedirectStatus redirectStatus)
static String stripURLForUseInReport(Document* document, const KURL& url, RedirectStatus redirectStatus, const String& effectiveDirective)
{
if (!url.isValid())
return String();
if (!url.isHierarchical() || url.protocolIs("file"))
return url.protocol();
if (redirectStatus == RedirectStatus::NoRedirect || document->getSecurityOrigin()->canRequest(url)) {
// Until we're more careful about the way we deal with navigations in frames (and, by extension,
// in plugin documents), strip cross-origin 'frame-src' and 'object-src' violations down to an
// origin. https://crbug.com/633306
bool canSafelyExposeURL = document->getSecurityOrigin()->canRequest(url)
|| (redirectStatus == RedirectStatus::NoRedirect
&& !equalIgnoringCase(effectiveDirective, ContentSecurityPolicy::FrameSrc)
&& !equalIgnoringCase(effectiveDirective, ContentSecurityPolicy::ObjectSrc));
if (canSafelyExposeURL) {
// 'KURL::strippedForUseAsReferrer()' dumps 'String()' for non-webby URLs.
// It's better for developers if we return the origin of those URLs rather
// than nothing.
......@@ -813,7 +822,7 @@ static void gatherSecurityPolicyViolationEventData(SecurityPolicyViolationEventI
init.setBlockedURI("eval");
break;
case ContentSecurityPolicy::URLViolation:
init.setBlockedURI(stripURLForUseInReport(document, blockedURL, redirectStatus));
init.setBlockedURI(stripURLForUseInReport(document, blockedURL, redirectStatus, effectiveDirective));
break;
}
}
......@@ -832,7 +841,7 @@ static void gatherSecurityPolicyViolationEventData(SecurityPolicyViolationEventI
std::unique_ptr<SourceLocation> location = SourceLocation::capture(document);
if (location->lineNumber()) {
KURL source = KURL(ParsedURLString, location->url());
init.setSourceFile(stripURLForUseInReport(document, source, redirectStatus));
init.setSourceFile(stripURLForUseInReport(document, source, redirectStatus, effectiveDirective));
init.setLineNumber(location->lineNumber());
init.setColumnNumber(location->columnNumber());
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment