mirror of
https://github.com/discourse/discourse.git
synced 2026-09-05 04:40:41 -05:00
SECURITY: Fix wildcard iframe origin allowlist to stop at URL authority boundaries
This commit is contained in:
committed by
Loïc Guitaut
parent
66653c484c
commit
d5ae094070
@@ -282,6 +282,32 @@ module("Unit | Utility | sanitizer", function (hooks) {
|
||||
);
|
||||
});
|
||||
|
||||
test("wildcard iframe origins stop at URL authority boundaries", function (assert) {
|
||||
const engine = build({
|
||||
siteSettings: { allowed_iframes: "https://*.example.com/" },
|
||||
});
|
||||
const iframe = (url) => `<iframe src="${url}"></iframe>`;
|
||||
|
||||
assert.strictEqual(
|
||||
engine.sanitize(iframe("https://deep.embed.example.com/player")),
|
||||
iframe("https://deep.embed.example.com/player"),
|
||||
"allows legitimate deep subdomains"
|
||||
);
|
||||
|
||||
[
|
||||
"https://attacker.example/@embed.example.com/player",
|
||||
"https://attacker.example?@embed.example.com/player",
|
||||
"https://attacker.example#@embed.example.com/player",
|
||||
"https://attacker.example\\@embed.example.com/player",
|
||||
].forEach((url) => {
|
||||
assert.strictEqual(
|
||||
engine.sanitize(iframe(url)),
|
||||
"",
|
||||
`rejects an iframe whose authority ends at ${url.match(/[/?#\\]/)[0]}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test("autoplay videos must be muted", function (assert) {
|
||||
let engine = build({ siteSettings: {} });
|
||||
assert.true(
|
||||
|
||||
@@ -146,7 +146,7 @@ export function sanitize(text, allowLister) {
|
||||
const regex = i
|
||||
// escape regex, keeping *
|
||||
.replace(/[.+?^${}()|[\]\\]/g, "\\$&")
|
||||
.replace(/\*/g, "[^/]+");
|
||||
.replace(/\*/g, "[^/?#\\\\]+");
|
||||
const allowedIframe = new RegExp(`^${regex}.*$`, "i");
|
||||
return iframeUrls.every((iframeUrl) =>
|
||||
allowedIframe.test(iframeUrl)
|
||||
|
||||
@@ -27,7 +27,7 @@ module Onebox
|
||||
origins.map do |origin|
|
||||
escaped_origin = Regexp.escape(origin)
|
||||
if origin.start_with?("*.", "https://*.", "http://*.")
|
||||
escaped_origin = escaped_origin.sub("\\*", "[^/?#]*")
|
||||
escaped_origin = escaped_origin.sub("\\*") { "[^/?#\\\\]*" }
|
||||
end
|
||||
|
||||
origin_boundary =
|
||||
|
||||
@@ -85,6 +85,7 @@ RSpec.describe Onebox::Engine do
|
||||
https://attacker.example/path.example.com/player
|
||||
https://attacker.example?.example.com/
|
||||
https://attacker.example#.example.com/
|
||||
https://attacker.example\\@embed.example.com/player
|
||||
https://embed.example.com.attacker.example/player
|
||||
].each { |url| expect(url).not_to match(regex) }
|
||||
end
|
||||
|
||||
@@ -2319,6 +2319,35 @@ RSpec.describe PostsController do
|
||||
expect(cooked.at_css(".onebox-attack")).to be_nil
|
||||
end
|
||||
|
||||
it "does not persist a backslash-bypassed wildcard iframe origin from oEmbed" do
|
||||
Jobs.run_immediately!
|
||||
url = "https://attacker.example.com/onebox"
|
||||
iframe_origin = "https://evil.example\\@sub.typeform.com/to/abc"
|
||||
|
||||
stub_request(:head, url).to_return(status: 200)
|
||||
stub_request(:get, url).to_return(
|
||||
status: 200,
|
||||
body:
|
||||
'<html><head><link type="application/json+oembed" href="https://attacker.example.com/oembed"></head></html>',
|
||||
)
|
||||
stub_request(:get, "https://attacker.example.com/oembed").to_return(
|
||||
status: 200,
|
||||
body: {
|
||||
title: "Attacker onebox",
|
||||
type: "rich",
|
||||
html: "<iframe src=\"#{iframe_origin}\"></iframe>",
|
||||
}.to_json,
|
||||
)
|
||||
|
||||
post "/posts.json", params: { raw: url, title: "Backslash iframe origin" }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.parsed_body["id"]).to be_present
|
||||
|
||||
cooked = Nokogiri::HTML5.fragment(Post.find(response.parsed_body["id"]).cooked)
|
||||
expect(cooked.at_css("iframe")).to be_nil
|
||||
end
|
||||
|
||||
it "creates the topic and post with the right attributes" do
|
||||
post "/posts.json",
|
||||
params: {
|
||||
|
||||
Reference in New Issue
Block a user