Skip to content

Commit 3815c1d

Browse files
rootvector2NullVoxPopuli
authored andcommitted
sanitize urls on svg anchor href and xlink:href
normalize tagName inside checkURI/checkDataURI fold related url sanitization fixes into this PR simplify checkURI to inline the tag normalization, and bring in the other open sanitization gaps so they live behind the same tag/attribute matching: - formaction on button/input - data: protocol on iframe[src] and object[data] - strip ascii tab/newline/cr before the fastboot url protocol check Sanitize javascript:/vbscript: urls in area[href] fix comment wording: svg tagNames are case-preserved
1 parent 8a9f078 commit 3815c1d

2 files changed

Lines changed: 116 additions & 8 deletions

File tree

packages/@glimmer-workspace/integration-tests/test/attributes-test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,54 @@ export class AttributesTests extends RenderTest {
573573
this.assertHTML('<svg viewBox="0 0 100 100" />');
574574
this.assertStableNodes();
575575
}
576+
577+
@test
578+
'svg a[href] marks javascript: protocol as unsafe'() {
579+
this.render('<svg><a href={{this.foo}}></a></svg>', { foo: 'javascript:foo()' });
580+
let anchor = (this.element.firstChild as SimpleElement).firstChild as SimpleElement;
581+
this.assert.strictEqual(this.readDOMAttr('href', anchor), 'unsafe:javascript:foo()');
582+
583+
this.rerender({ foo: 'http://foo.bar' });
584+
this.assert.strictEqual(this.readDOMAttr('href', anchor), 'http://foo.bar');
585+
}
586+
587+
@test
588+
'svg a[xlink:href] marks javascript: protocol as unsafe'() {
589+
this.render('<svg><a xlink:href={{this.foo}}></a></svg>', { foo: 'javascript:foo()' });
590+
let anchor = (this.element.firstChild as SimpleElement).firstChild as SimpleElement;
591+
this.assert.strictEqual(this.readDOMAttr('xlink:href', anchor), 'unsafe:javascript:foo()');
592+
593+
this.rerender({ foo: 'http://foo.bar' });
594+
this.assert.strictEqual(this.readDOMAttr('xlink:href', anchor), 'http://foo.bar');
595+
}
596+
597+
@test
598+
'marks data: urls as unsafe on iframe[src] and object[data]'() {
599+
this.render('<iframe src={{this.foo}}></iframe>', {
600+
foo: 'data:text/html,<script>alert(1)</script>',
601+
});
602+
this.assertHTML('<iframe src="unsafe:data:text/html,<script>alert(1)</script>"></iframe>');
603+
this.assertStableRerender();
604+
605+
this.rerender({ foo: 'https://example.com/page' });
606+
this.assertHTML('<iframe src="https://example.com/page"></iframe>');
607+
this.assertStableNodes();
608+
}
609+
610+
@test
611+
'object[data] marks data: and javascript: urls as unsafe but allows http'() {
612+
this.render('<object data={{this.foo}}></object>', {
613+
foo: 'data:text/html,<script>alert(1)</script>',
614+
});
615+
this.assertHTML('<object data="unsafe:data:text/html,<script>alert(1)</script>"></object>');
616+
617+
this.rerender({ foo: 'javascript:foo()' });
618+
this.assertHTML('<object data="unsafe:javascript:foo()"></object>');
619+
620+
this.rerender({ foo: 'https://example.com/doc.pdf' });
621+
this.assertHTML('<object data="https://example.com/doc.pdf"></object>');
622+
this.assertStableNodes();
623+
}
576624
}
577625

578626
jitSuite(AttributesTests);
@@ -719,3 +767,30 @@ jitSuite(
719767
protected isSelfClosing = false;
720768
}
721769
);
770+
771+
jitSuite(
772+
class extends BoundValuesToSpecialAttributeTests {
773+
static suiteName = 'button[formaction] attribute';
774+
protected tag = 'button';
775+
protected attr = 'formaction';
776+
}
777+
);
778+
779+
jitSuite(
780+
class extends BoundValuesToSpecialAttributeTests {
781+
static suiteName = 'input[formaction] attribute';
782+
protected tag = 'input';
783+
protected attr = 'formaction';
784+
protected override isEmptyElement = true;
785+
protected isSelfClosing = false;
786+
}
787+
);
788+
789+
jitSuite(
790+
class extends BoundValuesToSpecialAttributeTests {
791+
static suiteName = 'area[href] attribute';
792+
protected tag = 'area';
793+
protected attr = 'href';
794+
protected override isEmptyElement = true;
795+
}
796+
);

packages/@glimmer/runtime/lib/dom/sanitized-values.ts

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,51 @@ import { isSafeString, normalizeStringValue } from '../dom/normalize';
44

55
const badProtocols = ['javascript:', 'vbscript:'];
66

7-
const badTags = ['A', 'BODY', 'LINK', 'IMG', 'IFRAME', 'BASE', 'FORM'];
7+
const badTags = ['A', 'AREA', 'BODY', 'LINK', 'IMG', 'IFRAME', 'BASE', 'FORM', 'BUTTON', 'INPUT'];
88

99
const badTagsForDataURI = ['EMBED'];
1010

11-
const badAttributes = ['href', 'src', 'background', 'action'];
11+
// Tags whose URL attribute is loaded as a nested document. A `data:` URL there
12+
// is rendered and can execute script just like `javascript:`, so it has to be
13+
// neutralized even though such tags legitimately point at http(s) resources.
14+
const badTagsForDataProtocol = ['IFRAME', 'OBJECT'];
15+
16+
const badAttributes = ['href', 'src', 'background', 'action', 'formaction', 'xlink:href'];
1217

1318
const badAttributesForDataURI = ['src'];
1419

20+
const badAttributesForDataProtocol = ['src', 'data'];
21+
1522
function has(array: Array<string>, item: string): boolean {
1623
return array.indexOf(item) !== -1;
1724
}
1825

1926
function checkURI(tagName: Nullable<string>, attribute: string): boolean {
20-
return (tagName === null || has(badTags, tagName)) && has(badAttributes, attribute);
27+
// SVG tagNames are case-preserved, so the SVG `<a>` element comes through as
28+
// lowercase `a` and never matches the uppercase `badTags` entries unless we
29+
// normalize first.
30+
return (tagName === null || has(badTags, tagName.toUpperCase())) && has(badAttributes, attribute);
2131
}
2232

2333
function checkDataURI(tagName: Nullable<string>, attribute: string): boolean {
2434
if (tagName === null) return false;
25-
return has(badTagsForDataURI, tagName) && has(badAttributesForDataURI, attribute);
35+
return has(badTagsForDataURI, tagName.toUpperCase()) && has(badAttributesForDataURI, attribute);
36+
}
37+
38+
function checkDataProtocol(tagName: Nullable<string>, attribute: string): boolean {
39+
if (tagName === null) return false;
40+
return (
41+
has(badTagsForDataProtocol, tagName.toUpperCase()) &&
42+
has(badAttributesForDataProtocol, attribute)
43+
);
2644
}
2745

28-
export function requiresSanitization(tagName: string, attribute: string): boolean {
29-
return checkURI(tagName, attribute) || checkDataURI(tagName, attribute);
46+
export function requiresSanitization(tagName: Nullable<string>, attribute: string): boolean {
47+
return (
48+
checkURI(tagName, attribute) ||
49+
checkDataURI(tagName, attribute) ||
50+
checkDataProtocol(tagName, attribute)
51+
);
3052
}
3153

3254
interface NodeUrlParseResult {
@@ -63,7 +85,11 @@ function findProtocolForURL() {
6385
let protocol = null;
6486

6587
if (typeof url === 'string') {
66-
protocol = nodeURL.parse(url).protocol;
88+
// browsers strip ASCII tab/newline/CR from urls before navigating, so
89+
// `java\nscript:` runs as `javascript:`. `url.parse` keeps them and reports
90+
// a null protocol, slipping past the badProtocols check. Strip them here to
91+
// match the WHATWG `URL` parser used on the non-fastboot path.
92+
protocol = nodeURL.parse(url.replace(/[\t\n\r]/gu, '')).protocol;
6793
}
6894

6995
return protocol === null ? ':' : protocol;
@@ -108,7 +134,7 @@ export function sanitizeAttributeValue(
108134
return value.toHTML();
109135
}
110136

111-
const tagName = element.tagName.toUpperCase();
137+
const tagName = element.tagName;
112138

113139
let str = normalizeStringValue(value);
114140

@@ -119,6 +145,13 @@ export function sanitizeAttributeValue(
119145
}
120146
}
121147

148+
if (checkDataProtocol(tagName, attribute)) {
149+
let protocol = protocolForUrl(str);
150+
if (protocol === 'data:' || has(badProtocols, protocol)) {
151+
return `unsafe:${str}`;
152+
}
153+
}
154+
122155
if (checkDataURI(tagName, attribute)) {
123156
return `unsafe:${str}`;
124157
}

0 commit comments

Comments
 (0)