Re:Initial details about why CrowdStrike's CSAgent.sys crashed
by lolinder on Monday July 22, 2024 @07:36PM
(#41040944)
I think we've been talking past each other.
$result = $template.findNode("#foo").setText($unsanitizedInput)
This is not parsing the user input, this is letting the native API escape the input for you, which is exactly what I'm advocating for. See my note above:> escape the HTML using the language's standard library or the web framework's tooling.
This is what parsing the user input would look like with the DOM API:
const newDiv = document.createElement('div');
newDiv.innerHTML = untrustedUserInput;
// Do some work to attempt to sanitize the new HTML elements
document.body.appendChild(newDiv);
To me this is definitively a Bad Idea™, and I thought this was what you were advocating for.What you actually proposed is just escaping the HTML, not parsing user input, with the only twist being that you prefer to inject user input into your templating system imperatively with something resembling the DOM API instead of declaratively with something resembling JSX. That's fine, but not relevant to the question of what method we use to sanitize the untrusted input that we're injecting. On that front it sounds like we're in agreement that parsing user input is a terrible idea.
Working on the data structures after parsing makes it impossible to accidentally break the structure itself. Like, maybe your string escaping is perfect, but if you do:
Then you're still vulnerable to trivial errors in your template breaking the structure and creating an exploitable vulnerability, despite the $sanitizedString being correct. If you instead work at parsed level and do: Then there's just no way this can break (except bugs in the HTML parser and DOM API in general, which are much less likely to exist, and much easier to find and fix).