FIX: Apply 'allowed_href_schemes' to all src/srcset attributes (#16860)

Previously we were only applying the restriction to `a[href]` and `img[src]`. This commit ensures we apply the same logic to all allowlisted media src attributes.
This commit is contained in:
David Taylor 2022-05-19 11:18:30 +01:00 committed by GitHub
parent 95c85a278e
commit 166fe3bb34
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 76 additions and 7 deletions

View File

@ -176,6 +176,7 @@ export const DEFAULT_LIST = [
"img[title]",
"img[width]",
"img[data-thumbnail]",
// img[src] handled by sanitizer.js
"ins",
"kbd",
"li",
@ -203,14 +204,13 @@ export const DEFAULT_LIST = [
"sub",
"sup",
"source[data-orig-src]",
"source[src]",
"source[srcset]",
// source[src] and source[srcset] handled by sanitizer.js
"source[type]",
"track",
"track[default]",
"track[label]",
"track[kind]",
"track[src]",
// track[src] handled by sanitizer.js
"track[srclang]",
"ul",
"video",

View File

@ -41,6 +41,38 @@ export function hrefAllowed(href, extraHrefMatchers) {
}
}
function sanitizeMediaSrc(tag, attrName, value, extraHrefMatchers) {
const srcAttrs = {
img: ["src"],
source: ["src", "srcset"],
track: ["src"],
};
if (!srcAttrs[tag]?.includes(attrName)) {
return;
}
if (value.startsWith("data:image")) {
return attr(attrName, value);
}
if (attrName === "srcset") {
const srcset = value.split(",").map((v) => v.split(" ", 2));
const sanitizedValue = srcset
.map((src) => {
const allowedSrc = hrefAllowed(src[0], extraHrefMatchers);
if (allowedSrc) {
return src[1] ? `${allowedSrc} ${src[1]}` : allowedSrc;
}
})
.join(",");
return attr(attrName, sanitizedValue);
} else {
const returnVal = hrefAllowed(value, extraHrefMatchers);
return attr(attrName, returnVal);
}
}
function testDataAttribute(forTag, name, value) {
return Object.keys(forTag).find((k) => {
const nameWithMatcher = `^${k.replace(/\*$/, "\\w+?")}`;
@ -94,10 +126,6 @@ export function sanitize(text, allowLister) {
(tag === "a" &&
name === "href" &&
hrefAllowed(value, extraHrefMatchers)) ||
(tag === "img" &&
name === "src" &&
(/^data:image.*$/i.test(value) ||
hrefAllowed(value, extraHrefMatchers))) ||
(tag === "iframe" &&
name === "src" &&
allowedIframes.some((i) => {
@ -107,6 +135,16 @@ export function sanitize(text, allowLister) {
return attr(name, value);
}
const sanitizedMediaSrc = sanitizeMediaSrc(
tag,
name,
value,
extraHrefMatchers
);
if (sanitizedMediaSrc) {
return sanitizedMediaSrc;
}
if (tag === "iframe" && name === "src") {
return "-STRIP-";
}

View File

@ -1371,6 +1371,37 @@ describe PrettyText do
expect(cooked).to eq(n expected)
end
it "applies scheme restrictions to img[src] attributes" do
SiteSetting.allowed_href_schemes = "steam"
cooked = cook "![Steam URL Image](steam://store/452530) ![Other scheme image](itunes://store/452530)"
expected = '<p><img src="steam://store/452530" alt="Steam URL Image"> <img src="" alt="Other scheme image"></p>'
expect(cooked).to eq(n expected)
end
it "applies scheme restrictions to track[src] and source[src]" do
SiteSetting.allowed_href_schemes = "steam"
cooked = cook <<~MD
<video>
<source src="steam://store/452530"><source src="itunes://store/452530"><track src="steam://store/452530"><track src="itunes://store/452530">
</video>
MD
expect(cooked).to include <<~HTML
<source src="steam://store/452530"><source src=""><track src="steam://store/452530"><track src="">
HTML
end
it "applies scheme restrictions to source[srcset]" do
SiteSetting.allowed_href_schemes = "steam"
cooked = cook <<~MD
<video>
<source srcset="steam://store/452530 1x,itunes://store/123 2x"><source srcset="steam://store/452530"><source srcset="itunes://store/452530">
</video>
MD
expect(cooked).to include <<~HTML
<source srcset="steam://store/452530 1x,"><source srcset="steam://store/452530"><source srcset="">
HTML
end
it 'allows only tel URL scheme to start with a plus character' do
SiteSetting.allowed_href_schemes = "tel|steam"
cooked = cook("[Tel URL Scheme](tel://+452530579785)")