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

Misleading name/behavior for escapeHTML: Should denote "use for attribute values" #203

Open
mmichaelis opened this issue Oct 25, 2023 · 1 comment

Comments

@mmichaelis
Copy link

/**
* Replaces " to &qquot;
* @param {String} value
*/
const escapeHTML = (value) => value
.replace(/&/g, '&')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#039;')
// eslint-disable-next-line no-script-url
.replace(/(javascript|data|vbscript):/gi, '$1%3A');

The name escapeHTML suggests, that the method may be used to sanitize text-content and get rid of probably malicious nested HTML in BBCode, like [i]<script>javascript:alert("XSS!"</script>[/i]. Unfortunately, the method has an extra turn, to support escaping of probably unsafe href attributes: It also escapes problematic protocols assuming, we are in a URL-context.

Thus, naively reused in custom API the above will escape the text content to:

&lt;script&gt;javascript%3Aalert... (etc.)

The suggestion for clarity is to name the method escapeHTMLAttribute or, as this is considered breaking, at least mention this usage in the JSdoc.

Otherwise, I think the best option for escaping (and I tend to switch to it) is to rely on DOM processing as suggested in #148 (comment).

@JiLiZART
Copy link
Owner

DOM processing is not possible because this library is isomorphic. But you can escape html attributes in your own plugin using DOM API.
I have ideas to extract this function to separate folder with browser.js and node.js version (using platform API like DOM or node js builtin functions)

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

No branches or pull requests

2 participants