Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It is possible to add a disallowed closing tag with valid HTML as well as opening tag with invalid HTML markup. #549

Closed
ghost opened this issue May 3, 2022 · 3 comments · Fixed by #568
Labels

Comments

@ghost
Copy link

ghost commented May 3, 2022

I found several variants of the library's incorrect behavior. In the examples below, it is possible to add any html tag (closing tag with valid HTML as well as opening tag with invalid HTML) if any tag is allowed.

Example 1:

const sanitizeHtml = require('sanitize-html');

const sanitizedString = sanitizeHtml('<b><div/', {
    allowedTags: ['b'],
});

Expected behavior

As a result of the execution I expect to see <b></b> or a empty line. However, I get <b></div>.

Example 2:

const sanitizeHtml = require('sanitize-html');
const HTMLParser = require('node-html-parser');

const sanitizedString = sanitizeHtml('<b><b<<div/', {
    allowedTags: ['b'],
});

console.log(sanitizedString) // <b></b<<div>

const unespectedDiv = HTMLParser.parse(sanitizedString).querySelector('div');

console.log(unespectedDiv);

Expected behavior

As a result of the execution I expect to see <b></b> or a empty line. However, I get <b></b<<div>. The resulting string contains a substring <div>, which is interpreted by some parsers as a valid html tag like node-html-parser (Browsers interpret it correctly).

@ghost ghost added the bug label May 3, 2022
@boutell
Copy link
Member

boutell commented May 3, 2022

Since browsers interpret it correctly I don't regard it as a security issue, but I agree it is not ideal output. It would make more sense for the additional <'s to get escaped or something.

@boutell
Copy link
Member

boutell commented May 3, 2022 via email

@ghost
Copy link
Author

ghost commented May 4, 2022

I completely agree with you and it is not a security risk, but in some scenarios it can lead to unexpected results. However, I'm not sure I'm the best candidate for fixing this behavior. I just wanted to share some research.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant