Skip to content

Commit

Permalink
fix: escape matching closing tag in raw text
Browse files Browse the repository at this point in the history
This commit adds a logic to escape a matching closing tag in raw text.

For example, given `<style>#text(</style><script></script>)</style>`, the parent tag would by "style" and the raw text is "</style><script></script>". If we come across a matching closing tag (in out case `</style>`) - replace `<` with `&lt;` to avoid unexpected and unsafe behavior after de-serialization.
  • Loading branch information
AndrewKushnir committed Sep 28, 2023
1 parent 9212032 commit ad21ca6
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 1 deletion.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ var domino = require('domino');
Domino includes test from the [W3C DOM Conformance Suites](http://www.w3.org/DOM/Test/)
as well as tests from [HTML Working Group](http://www.w3.org/html/wg/wiki/Testing).

When you checkout this repository for the first time, run the following command to also check out code for the mentioned tests:

```
git submodule update --init --recursive
```

The tests can be run via `npm test` or directly though the [Mocha](http://mochajs.org/) command line:

![Screenshot](http://fgnass.github.com/images/domino.png)
Expand Down
53 changes: 52 additions & 1 deletion lib/NodeUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,49 @@ function attrname(a) {
return a.name;
}

/**
* Escapes matching closing tag in a raw text.
*
* For example, given `<style>#text(</style><script></script>)</style>`,
* the parent tag would by "style" and the raw text is
* "</style><script></script>". If we come across a matching closing tag
* (in out case `</style>`) - replace `<` with `&lt;` to avoid unexpected
* and unsafe behavior after de-serialization.
*/
function escapeMatchingClosingTag(rawText, parentTag) {
let result = '';
let i = 0;
let isInCommentBlock = false;
const lowercasedRawText = rawText.toLowerCase();
const pattern = '</' + parentTag.toLowerCase() + '>';
while (i < rawText.length) {
let currentChar = rawText[i];
if (isInCommentBlock) {
if (rawText.substr(i, 3) === '-->') {
// We came across a closing comment tag while being in
// a comment block. Set the flag and proceed to the next char.
isInCommentBlock = false;
}
} else {
if (rawText.substr(i, 4) === '<!--') {
// We came across an open comment tag,
// set the flag and proceed to the next char.
isInCommentBlock = true;
} else {
// We are not in comment block, check if we have
// a lookup pattern in front of us. Compare against
// a lowercased content to avoid case sensitivity.
if (lowercasedRawText.substr(i, pattern.length) === pattern) {
currentChar = '&lt;'; // replace '<' with '&lt;'
}
}
}
result += currentChar;
i++;
}
return result;
}

function serializeOne(kid, parent) {
var s = '';
switch(kid.nodeType) {
Expand Down Expand Up @@ -134,7 +177,15 @@ function serializeOne(kid, parent) {

if (hasRawContent[parenttag] ||
(parenttag==='NOSCRIPT' && parent.ownerDocument._scripting_enabled)) {
s += kid.data;
if (parenttag === 'PLAINTEXT') {
// Exclude <plaintext> tag from escaping, since it breaks parsing-related
// tests in test/html5lib-tests (external tests). The <plaintext> is deprecated
// (see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/plaintext) and
// not supported by Angular (Angular throws an error).
s += kid.data;
} else {
s += escapeMatchingClosingTag(kid.data, parenttag);
}
} else {
s += escape(kid.data);
}
Expand Down
63 changes: 63 additions & 0 deletions test/xss.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,66 @@ exports.fp170_37 = function() {
'<p><svg><style>*{font-family:\'&lt;/style&gt;&lt;img/src=x\tonerror=xss()//\'}</style></svg></p>'
);
};

exports.styleMatchingClosingTagInRawText = function() {
const document = domino.createDocument('');
const style = document.createElement("style");
style.textContent = "abc</style><script>alert(1)</script>";
document.body.appendChild(style);

// Ensure that HTML entities are properly encoded inside <style>
document.body.serialize().should.equal(
'<style>abc&lt;/style><script>alert(1)</script></style>'
);
};

exports.styleMatchingClosingTagSkipsCommentedContent = function() {
const document = domino.createDocument('');
const style = document.createElement("style");
style.textContent = "abc<!--</style>--><script>alert(1)</script>";
document.body.appendChild(style);

// No escaping in this case, since </style> is inside a comment block.
document.body.serialize().should.equal(
'<style>abc<!--</style>--><script>alert(1)</script></style>'
);
};

exports.styleMatchingClosingTagAfterClosingComment = function() {
const document = domino.createDocument('');
const style = document.createElement("style");
style.textContent = "abc--></style><script>alert(1)</script>";
document.body.appendChild(style);

// Ensure that HTML entities are properly encoded inside <style>
document.body.serialize().should.equal(
'<style>abc-->&lt;/style><script>alert(1)</script></style>'
);
};

exports.styleMatchingClosingTagSkipsUnclosedCommentedContent = function() {
const document = domino.createDocument('');
const style = document.createElement("style");
style.textContent = "abc<!--</style><script>alert(1)</script>";
document.body.appendChild(style);

// No escaping in this case, since </style> is located after
// an open comment tag (thus the contents after it would remain
// in a comment block after parsing).
document.body.serialize().should.equal(
'<style>abc<!--</style><script>alert(1)</script></style>'
);
};

exports.scriptMatchingClosingTagInRawText = function() {
const document = domino.createDocument('');
const script = document.createElement("script");
script.textContent = "abc</script><script>alert(1)</script>";
document.body.appendChild(script);

// Ensure that HTML entities are properly encoded inside <script>
// Note: the `</script>` is encoded in both places.
document.body.serialize().should.equal(
'<script>abc&lt;/script><script>alert(1)&lt;/script></script>'
);
};

0 comments on commit ad21ca6

Please sign in to comment.