String#escapeHTML() only encodes &, < and >. It does not encode quote characters, so a value placed inside a quoted HTML attribute can still break out of that attribute, which leads to XSS even after the value has been "escaped".
Current implementation (around line 421 of src/prototype/lang/string.js):
function escapeHTML() {
return this.replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>');
}Reproduction:
var s = 'x" onmouseover="alert(document.domain)';
var html = '<a title="' + s.escapeHTML() + '">hi</a>';
// produces: <a title="x" onmouseover="alert(document.domain)">hi</a>
// the double quote is not encoded, so an event handler gets injected
The method name implies it is safe for HTML, and a lot of code uses it to build attribute values, so this gap is easy to miss.
Suggested fix: also encode " to " and ' to ' (encoding / as well does no harm). If the intent is that the method is only ever safe for text node content, that is worth stating clearly in the docs, along with a separate attribute-safe helper.
Refs: CWE-79, CWE-116, OWASP XSS Prevention Cheat Sheet (rule #2), OWASP ASVS V5.3.1 / V5.3.3.
String#escapeHTML()only encodes&,<and>. It does not encode quote characters, so a value placed inside a quoted HTML attribute can still break out of that attribute, which leads to XSS even after the value has been "escaped".Current implementation (around line 421 of
src/prototype/lang/string.js):Reproduction:
The method name implies it is safe for HTML, and a lot of code uses it to build attribute values, so this gap is easy to miss.
Suggested fix: also encode
"to"and'to'(encoding/as well does no harm). If the intent is that the method is only ever safe for text node content, that is worth stating clearly in the docs, along with a separate attribute-safe helper.Refs: CWE-79, CWE-116, OWASP XSS Prevention Cheat Sheet (rule #2), OWASP ASVS V5.3.1 / V5.3.3.