Skip to content

Commit 87221d4

Browse files
authored
Merge pull request #1282 from Patternslib/thet/pat-validation/sanitize-error-msg
feat(pat-validation): Sanitize the validation message.
2 parents 0d042b1 + 87db300 commit 87221d4

2 files changed

Lines changed: 268 additions & 10 deletions

File tree

src/pat/validation/validation.js

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,26 @@ class Pattern extends BasePattern {
4242
// validation fails (e.g. pat-inject).
4343
static order = 100;
4444

45+
_dompurify = null;
46+
async get_dompurify() {
47+
if (!this._dompurify) {
48+
this._dompurify = (await import("dompurify")).default;
49+
}
50+
return this._dompurify;
51+
}
52+
4553
init() {
4654
events.add_event_listener(
4755
this.el,
4856
"submit",
4957
`pat-validation--submit--validator`,
50-
(e) => {
58+
async (e) => {
5159
// On submit, check all.
5260
// Immediate, non-debounced check with submit. Otherwise submit
5361
// is not cancelable.
5462
for (const input of this.inputs) {
5563
logger.debug("Checking input for submit", input, e);
56-
this.check_input({ input: input, event: e });
64+
await this.check_input({ input: input, event: e });
5765
}
5866
},
5967
// Make sure this event handler is run early, in the capturing
@@ -108,7 +116,7 @@ class Pattern extends BasePattern {
108116
}
109117
}
110118

111-
check_input({ input, event, stop = false }) {
119+
async check_input({ input, event, stop = false }) {
112120
if (input.disabled) {
113121
// No need to check disabled inputs.
114122
return;
@@ -239,11 +247,11 @@ class Pattern extends BasePattern {
239247
// do not re-check when stop is set to avoid infinite loops
240248
if (!stop && not_after_el) {
241249
logger.debug("Check `not-after` input.", not_after_el);
242-
this.check_input({ input: not_after_el, stop: true });
250+
await this.check_input({ input: not_after_el, stop: true });
243251
}
244252
if (!stop && not_before_el) {
245253
logger.debug("Check `no-before` input.", not_after_el);
246-
this.check_input({ input: not_before_el, stop: true });
254+
await this.check_input({ input: not_before_el, stop: true });
247255
}
248256
}
249257

@@ -318,7 +326,7 @@ class Pattern extends BasePattern {
318326
event.stopPropagation();
319327
event.stopImmediatePropagation();
320328
}
321-
this.set_error_message(input);
329+
await this.set_error_message(input);
322330
}
323331

324332
set_error({
@@ -381,7 +389,7 @@ class Pattern extends BasePattern {
381389
}
382390
}
383391

384-
set_error_message(input) {
392+
async set_error_message(input) {
385393
// First, remove the old error message.
386394
this.remove_error(input, false, true);
387395

@@ -393,8 +401,16 @@ class Pattern extends BasePattern {
393401
return;
394402
}
395403

396-
// Create the validation error DOM node from the template
397-
const validation_message = input.validationMessage || input[KEY_ERROR_MSG];
404+
// Create the validation error DOM node from the template.
405+
// Sanitize the validation message to keep malicious input from being
406+
// executed. Chrome includes the input value in it's browser validation
407+
// message. When placing that into the DOM, malicious input could get
408+
// executed within the web page context.
409+
const dompurify = await this.get_dompurify();
410+
const validation_message = dompurify.sanitize(
411+
input.validationMessage || input[KEY_ERROR_MSG]
412+
);
413+
398414
const error_node = dom.create_from_string(
399415
this.error_template(validation_message)
400416
).firstChild;
@@ -431,7 +447,7 @@ class Pattern extends BasePattern {
431447
if (did_disable) {
432448
logger.debug("Checking whole form after element was disabled.");
433449
for (const _input of this.inputs.filter((it) => it !== input)) {
434-
this.check_input({ input: _input, stop: true });
450+
await this.check_input({ input: _input, stop: true });
435451
}
436452
}
437453
}

src/pat/validation/validation.test.js

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ describe("pat-validation", function () {
458458
await events.await_pattern_init(instance);
459459

460460
document.querySelector("button").click();
461+
await utils.timeout(1); // wait a tick for async to settle.
461462

462463
expect(el.querySelectorAll("em.warning").length).toBe(2);
463464
});
@@ -1509,4 +1510,245 @@ describe("pat-validation", function () {
15091510
expect(el.querySelector("#form-buttons-create").disabled).toBe(false);
15101511
});
15111512
});
1513+
1514+
describe("8 - security tests", function () {
1515+
it("8.1 - sanitizes malicious script content in validation messages", async function () {
1516+
document.body.innerHTML = `
1517+
<form class="pat-validation">
1518+
<input type="text" name="malicious" required>
1519+
</form>
1520+
`;
1521+
const el = document.querySelector(".pat-validation");
1522+
const inp = el.querySelector("[name=malicious]");
1523+
1524+
const instance = new Pattern(el);
1525+
await events.await_pattern_init(instance);
1526+
1527+
// Use the pattern's set_error method which would eventually call set_error_message
1528+
// This simulates setting a custom error message with malicious content
1529+
const malicious_message =
1530+
"Please fill out this field: <script>alert(33)</script>";
1531+
instance.set_error({ input: inp, msg: malicious_message });
1532+
1533+
// Manually trigger set_error_message to test the sanitization
1534+
await instance.set_error_message(inp);
1535+
1536+
// Check that an error message is shown
1537+
expect(el.querySelectorAll("em.warning").length).toBe(1);
1538+
1539+
// Verify the script tag has been sanitized/removed
1540+
const warning_text = el.querySelector("em.warning").textContent;
1541+
expect(warning_text).not.toContain("<script>");
1542+
expect(warning_text).not.toContain("</script>");
1543+
expect(warning_text).not.toContain("alert(33)");
1544+
1545+
// The sanitized message should still contain the safe text
1546+
expect(warning_text).toContain("Please fill out this field");
1547+
1548+
// Ensure no script elements were actually added to the DOM
1549+
expect(el.querySelectorAll("script").length).toBe(0);
1550+
});
1551+
1552+
it("8.2 - sanitizes malicious HTML content in validation messages", async function () {
1553+
document.body.innerHTML = `
1554+
<form class="pat-validation">
1555+
<input type="text" name="malicious" required>
1556+
</form>
1557+
`;
1558+
const el = document.querySelector(".pat-validation");
1559+
const inp = el.querySelector("[name=malicious]");
1560+
1561+
const instance = new Pattern(el);
1562+
await events.await_pattern_init(instance);
1563+
1564+
// Set malicious HTML content that could execute JavaScript
1565+
const malicious_message =
1566+
'Please fill out this field: <img src="x" onerror="alert(33)">';
1567+
instance.set_error({ input: inp, msg: malicious_message });
1568+
1569+
// Manually trigger set_error_message to test the sanitization
1570+
await instance.set_error_message(inp);
1571+
1572+
// Check that an error message is shown
1573+
expect(el.querySelectorAll("em.warning").length).toBe(1);
1574+
1575+
// Verify the malicious HTML has been sanitized
1576+
const warning_element = el.querySelector("em.warning");
1577+
const warning_text = warning_element.textContent;
1578+
const warning_html = warning_element.innerHTML;
1579+
1580+
// The text should not contain the malicious content
1581+
expect(warning_text).not.toContain("onerror");
1582+
expect(warning_text).not.toContain("alert(33)");
1583+
1584+
// The HTML should also be sanitized - the dangerous onerror attribute should be removed
1585+
// DOMPurify keeps safe <img> tags but removes dangerous event handlers
1586+
expect(warning_html).not.toContain("onerror");
1587+
expect(warning_html).not.toContain("alert(33)");
1588+
1589+
// The safe text should remain
1590+
expect(warning_text).toContain("Please fill out this field");
1591+
1592+
// Ensure no img elements with onerror were added
1593+
const imgs_with_onerror = document.querySelectorAll("img[onerror]");
1594+
expect(imgs_with_onerror.length).toBe(0);
1595+
});
1596+
1597+
it("8.3 - preserves safe HTML content in validation messages", async function () {
1598+
document.body.innerHTML = `
1599+
<form class="pat-validation">
1600+
<input type="text" name="safe" required>
1601+
</form>
1602+
`;
1603+
const el = document.querySelector(".pat-validation");
1604+
const inp = el.querySelector("[name=safe]");
1605+
1606+
const instance = new Pattern(el);
1607+
await events.await_pattern_init(instance);
1608+
1609+
// Set a validation message with safe HTML content (like emphasis)
1610+
const safe_message =
1611+
"This field is <strong>required</strong> and must be filled out.";
1612+
instance.set_error({ input: inp, msg: safe_message });
1613+
1614+
// Trigger set_error_message
1615+
await instance.set_error_message(inp);
1616+
1617+
// Check that an error message is shown
1618+
expect(el.querySelectorAll("em.warning").length).toBe(1);
1619+
1620+
const warning_element = el.querySelector("em.warning");
1621+
const warning_text = warning_element.textContent;
1622+
const warning_html = warning_element.innerHTML;
1623+
1624+
// The text content should contain the message without HTML tags
1625+
expect(warning_text).toContain(
1626+
"This field is required and must be filled out."
1627+
);
1628+
1629+
// The innerHTML should contain the safe HTML (DOMPurify allows <strong> by default)
1630+
expect(warning_html).toContain("<strong>required</strong>");
1631+
});
1632+
1633+
it("8.4 - handles validation messages from browser's built-in validation", async function () {
1634+
document.body.innerHTML = `
1635+
<form class="pat-validation">
1636+
<input type="email" name="email" required>
1637+
</form>
1638+
`;
1639+
const el = document.querySelector(".pat-validation");
1640+
const inp = el.querySelector("[name=email]");
1641+
1642+
const instance = new Pattern(el);
1643+
await events.await_pattern_init(instance);
1644+
1645+
// Set an invalid email that would trigger browser validation
1646+
// and potentially include the malicious input in the validation message
1647+
inp.value = '<script>alert("xss")</script>@example.com';
1648+
1649+
// Trigger validation
1650+
inp.dispatchEvent(events.change_event());
1651+
await utils.timeout(1); // wait a tick for async to settle.
1652+
1653+
// Check that an error message is shown
1654+
expect(el.querySelectorAll("em.warning").length).toBe(1);
1655+
1656+
const warning_text = el.querySelector("em.warning").textContent;
1657+
1658+
// Verify the script content has been sanitized from the validation message
1659+
expect(warning_text).not.toContain("<script>");
1660+
expect(warning_text).not.toContain("</script>");
1661+
expect(warning_text).not.toContain('alert("xss")');
1662+
1663+
// Ensure no script elements were added to the DOM
1664+
expect(el.querySelectorAll("script").length).toBe(0);
1665+
});
1666+
1667+
it("8.5 - sanitizes example malicious script that would execute", async function () {
1668+
document.body.innerHTML = `
1669+
<form class="pat-validation">
1670+
<input type="text" name="example" required>
1671+
</form>
1672+
`;
1673+
const el = document.querySelector(".pat-validation");
1674+
const inp = el.querySelector("[name=example]");
1675+
1676+
const instance = new Pattern(el);
1677+
await events.await_pattern_init(instance);
1678+
1679+
// Set the exact example from the user's request
1680+
const malicious_message = "Value is invalid: <script>alert(33)</script>";
1681+
instance.set_error({ input: inp, msg: malicious_message });
1682+
1683+
// Trigger set_error_message to test the sanitization
1684+
await instance.set_error_message(inp);
1685+
1686+
// Check that an error message is shown
1687+
expect(el.querySelectorAll("em.warning").length).toBe(1);
1688+
1689+
// Verify the script content has been completely sanitized
1690+
const warning_element = el.querySelector("em.warning");
1691+
const warning_text = warning_element.textContent;
1692+
const warning_html = warning_element.innerHTML;
1693+
1694+
// Verify no script execution
1695+
expect(warning_text).not.toContain("<script>");
1696+
expect(warning_text).not.toContain("</script>");
1697+
expect(warning_text).not.toContain("alert(33)");
1698+
1699+
// HTML should also be clean
1700+
expect(warning_html).not.toContain("<script>");
1701+
expect(warning_html).not.toContain("</script>");
1702+
1703+
// The safe part of the message should remain
1704+
expect(warning_text).toContain("Value is invalid");
1705+
1706+
// Ensure no script elements were added to the DOM
1707+
expect(document.querySelectorAll("script").length).toBe(0);
1708+
});
1709+
1710+
it("8.6 - sanitizes malicious content in browser validationMessage property", async function () {
1711+
document.body.innerHTML = `
1712+
<form class="pat-validation">
1713+
<input type="text" name="browser" required>
1714+
</form>
1715+
`;
1716+
const el = document.querySelector(".pat-validation");
1717+
const inp = el.querySelector("[name=browser]");
1718+
1719+
const instance = new Pattern(el);
1720+
await events.await_pattern_init(instance);
1721+
1722+
// Simulate browser validation message with malicious content
1723+
// This could happen if browser includes user input in validation message
1724+
inp.value = "";
1725+
1726+
// Mock the validationMessage property to contain malicious content
1727+
// This simulates what Chrome might do
1728+
Object.defineProperty(inp, "validationMessage", {
1729+
value: 'Please fill out this field. Input: <script>alert("malicious")</script>',
1730+
configurable: true,
1731+
});
1732+
1733+
// Trigger validation through the pattern
1734+
instance.check_input({ input: inp });
1735+
await utils.timeout(1); // wait for async to settle.
1736+
1737+
// Check that an error message is shown
1738+
expect(el.querySelectorAll("em.warning").length).toBe(1);
1739+
1740+
const warning_text = el.querySelector("em.warning").textContent;
1741+
1742+
// Verify the script has been sanitized
1743+
expect(warning_text).not.toContain("<script>");
1744+
expect(warning_text).not.toContain("</script>");
1745+
expect(warning_text).not.toContain('alert("malicious")');
1746+
1747+
// Safe content should remain
1748+
expect(warning_text).toContain("Please fill out this field");
1749+
1750+
// No scripts should be in the DOM
1751+
expect(document.querySelectorAll("script").length).toBe(0);
1752+
});
1753+
});
15121754
});

0 commit comments

Comments
 (0)