Skip to content

Commit 2b78613

Browse files
Merge pull request #21479 from ubeddulla/sanitize-url-attr-case
sanitize url attributes case-insensitively
2 parents 9834747 + d8420f7 commit 2b78613

2 files changed

Lines changed: 24 additions & 4 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,18 @@ export class AttributesTests extends RenderTest {
625625
this.assertHTML(`<object data="${allowedUrl}"></object>`);
626626
this.assertStableNodes();
627627
}
628+
629+
@test
630+
'marks javascript: protocol as unsafe on a camelCased url attribute'() {
631+
// `formAction` resolves to the DOM property, so the attribute name reaches
632+
// the sanitizer camelCased rather than as the lowercase `formaction`.
633+
this.render('<button formAction={{this.foo}}></button>', { foo: 'javascript:foo()' });
634+
let button = this.element.firstChild as SimpleElement;
635+
this.assert.strictEqual(this.readDOMAttr('formAction', button), 'unsafe:javascript:foo()');
636+
637+
this.rerender({ foo: 'http://foo.bar/derp' });
638+
this.assert.strictEqual(this.readDOMAttr('formAction', button), 'http://foo.bar/derp');
639+
}
628640
}
629641

630642
jitSuite(AttributesTests);

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,28 @@ function has(array: Array<string>, item: string): boolean {
1919
function checkURI(tagName: Nullable<string>, attribute: string): boolean {
2020
// SVG tagNames are case-preserved, so the SVG `<a>` element comes through as
2121
// lowercase `a` and never matches the uppercase `badTags` entries unless we
22-
// normalize first.
23-
return (tagName === null || has(badTags, tagName.toUpperCase())) && has(badAttributes, attribute);
22+
// normalize first. The attribute name can likewise arrive camelCased (e.g.
23+
// `formAction`) when the template author writes it that way and it resolves to
24+
// a DOM property, so lower-case it before matching the lowercase lists.
25+
return (
26+
(tagName === null || has(badTags, tagName.toUpperCase())) &&
27+
has(badAttributes, attribute.toLowerCase())
28+
);
2429
}
2530

2631
function checkDataURI(tagName: Nullable<string>, attribute: string): boolean {
2732
if (tagName === null) return false;
28-
return has(badTagsForDataURI, tagName.toUpperCase()) && has(badAttributesForDataURI, attribute);
33+
return (
34+
has(badTagsForDataURI, tagName.toUpperCase()) &&
35+
has(badAttributesForDataURI, attribute.toLowerCase())
36+
);
2937
}
3038

3139
function checkDataProtocol(tagName: Nullable<string>, attribute: string): boolean {
3240
if (tagName === null) return false;
3341
return (
3442
has(badTagsForDataProtocol, tagName.toUpperCase()) &&
35-
has(badAttributesForDataProtocol, attribute)
43+
has(badAttributesForDataProtocol, attribute.toLowerCase())
3644
);
3745
}
3846

0 commit comments

Comments
 (0)