3 ms·

That's all mitigated on the server. You also have access to additional escaping tools for handling user-generated data on the server, too.

chrismorgan 2y ago
> That's all mitigated on the server. How? Please be specific, because unless you're altering the source (which I'd be very unpleasantly shocked by), your use of untagged template strings makes it fundamentally impossible to mitigate such stuff, short of forbidding characters like & and < and " from data, which would be insanity. You're mixing template and data willy-nilly.
- rglover 2y ago
  The source isn't altered (I take the component HTML, parse it, build it into a DOM, and render it). The data from the API is compiled separately (where sanitization can happen if you have untrusted user data). The two are combined together during SSR by passing the sanitized data to the component. Developer controlled code at the component-level is assumed trusted, however, there are three mechanisms for handling escaping/sanitizing on the server: 1. There are helpers built in to the API (you can conditionally enable sanitization depending on need) to handle sanitization and helper functions built-in to the server-side to do one-off escaping/sanitization for HTML. 2. Query params are sanitized automatically to prevent injection. 3. To prevent unescaped HTML in data from breaking renders, I base64 encode it before sending it to the browser. This approach to escaping is by design as some use cases call for sanitization (they're rendering user-generated data) while others do not. You're welcome to audit the source and see if I missed something or make suggestions on improvements.
  - chrismorgan 2y ago
    I wrote something much longer and more detailed, but I haven’t the heart to post it in the end. This is a phenomenally bad idea. By escaping things willy-nilly you’re making life much harder, and vastly more error-prone. You can only even vaguely sanely do such things with the support of a better type system than you have available to you. You’re encouraging storing escaped HTML in the database; exposing an API that HTML-encodes all strings; you’re misusing the term “sanitise” (sanitising is fairly consistently used to mean stripping control sequences, not escaping). This is a disaster. Given that you’re working in JavaScript, you need to use something like JSX or equivalent functions or tagged template strings, so you can escape only as appropriate. As it stands, I would genuinely expect to be able to find trivial functionality and/or security bugs in practically every decent-sized Joystick site involving user input. One specific technical bug: https://github.com/cheatcode/joystick/blob/canary/node/src/lib/escape_html.js https://github.com/cheatcode/joystick/blob/canary/node/src/l... tries to escape &<>"'`= but will replace backticks and equals signs with “undefined”. Defining HTML_ENTITY_MAP in a separate file was a mistake; you’ll hopefully never refer to it anywhere else, and separating it made such an error easier to make. (Incidentally, backtick and equals sign have never been useful things to escape in HTML. Truthfully, & and < are all you need in HTML text, and & and " in double-quoted attribute value. If you want to support XML, then escape < and > everywhere as well. But nothing beyond that is useful.)
    - rglover 2y ago
      I updated the escape_html() method to cover the missing characters/cases and centralized the entity map to the function definition. Appreciate you pointing that out. I went beyond what you suggested in terms of characters which should cover most (if not all) problem cases. Here's an example repo: https://github.com/cheatcode/escape_test https://github.com/cheatcode/escape_test And example text I used to test: https://gist.github.com/rglover/c75e37b526d864313b461d38195b6970 https://gist.github.com/rglover/c75e37b526d864313b461d38195b... > You’re encouraging storing escaped HTML in the database You can store unescaped HTML in the database and upon retrieval, escape it (see the linked demo repo above). You hypothetically could store escaped HTML, but you'd have to show me where I'm encouraging it (I think what you're saying is that because you can do it, I'm encouraging it). > As it stands, I would genuinely expect to be able to find trivial functionality and/or security bugs in practically every decent-sized Joystick site involving user input. You're welcome to audit it, but please, actually test it by building something and then provide an example repo. Make sure to use the canary release as that has the latest changes. If you email me (ryan.glover@cheatcode.co) I can give specifics on how to get set up. --- FWIW, this isn't a productive way to point out issues with a project. A PR with comments is preferred.
      - chrismorgan 2y ago
        Sorry, but it’s like the reaction to https://news.ycombinator.com/item?id=42359067 https://news.ycombinator.com/item?id=42359067: “this is neither a new idea nor a good one”. Ad-hoc escaping for serialisation is just fundamentally a really bad approach, and although it used to be how almost everything did it, nothing serious has tried working that way for at least a decade, probably closer to two by this point, because as an industry we’ve actually learned things. Your particular angle on it might be novel, I’m not certain, but only because it’s an even worse idea. Formerly, there was a simple rule: remember to escape data when emitting HTML. You’ve replaced that with something complex: some things will be entity-encoded, and others won’t; and often you won’t want things entity-encoded. (You’re inappropriately tying API to HTML, by the looks of it.) This is just a bad abstraction that makes errors sure, and errors will lead to some mangled text in all cases, and security bugs in some cases. It’s very similar to how the consensus is now well-established (though it’ll take plenty more time to be fully applied) in memory safety, a related security field: avoid languages like C, they’re too dangerous, use memory-safe languages instead. Another well-established principle: parse at input, let your system deal with data, and serialise at output. Your approach instead serialises at input—and that only most of the time—and hopes you never need to parse. I’m certain that I’ve upset you, and I’m sorry about that, but the approach really, truly is that bad, and I did just want to appeal to you to think it over. Look, if you can find someone experienced in web security and frontend framework design and things like that, preferably who’s been doing this for fifteen or twenty years, present the whole design to them, and see what they say. I genuinely expect horror. As for your changes to escape_html, I’ll just ask why? because you’ve compounded the error. There is no plausible purpose for all the additions you’ve made. By encoding, you declare intent to feed the value to an HTML or XML parser in data state, single-quoted attribute state or double-quoted attribute state. In none of those states do any of these additions achieve anything. And if you’re making them for some other purpose, then you don’t want the HTML escaping in the first place. Fifteen years ago, I would encounter both double-escaped HTML and entity-encoding in non-HTML contexts, far too often. Now, I feel like it’s years since I’ve seen such a thing, outside of text/plain parts of emails (but developers and marketers are frequently awful about text/plain parts). The only advantage I can see in entity-encoding more characters, especially all normal punctuation, is that bad escaping is going to be caught earlier. But I say, if you reckon those worth escaping, why not go the whole hog and .replace(/./ug, s => '&#' + s.codePointAt(0) + ';')? I’m not sure what your experience is, but the impression I’m getting is that you’ve been largely just JavaScript. A lot of the problems you’re producing were best-known through PHP, and better type systems are a large part of the best solutions to these things. If I’m sounding you out correctly, I’d recommend that you look into and learn some strong statically-typed language, preferably one with algebraic data types (also called sum types). Rust has been my own preference for the last decade, but there are plenty of others that would guide you in similar lessons, like (in no particular order) C♯, Swift, Elixir, Kotlin, Scala, Haskell. (I don’t include TypeScript in this list, although it could definitely help with some of the lessons, because it’s too compromised by JavaScript limitations.) If you haven’t had any of this sort of experience, getting it will help you to write better JavaScript, and make better systems. —⁂— Now, for a specific matter: > You hypothetically could store escaped HTML, but you'd have to show me where I'm encouraging it <https://docs.cheatcode.co/joystick/node/escape_html#:~:text=The%20escape_html()%20method%20is%20helpful%20for%20pre-sanitizing%20user%20input%20before%20it%20hits%20your%20database. https://docs.cheatcode.co/joystick/node/escape_html#:~:text=...> (And if you unconditionally entity-encode query string parameters… I don’t even want to think about the implications of that.) I would also point out that https://docs.cheatcode.co/joystick/node/app/api/sanitization https://docs.cheatcode.co/joystick/node/app/api/sanitization says: > Sanitization is the process of scrubbing unexpected or unwanted HTML tags from the values returned by your API. But this is not what you’ve implemented. You don’t actually scrub, you entity-encode.

chrismorgan 2y ago

> That's all mitigated on the server. How? Please be specific, because unless you're altering the source (which I'd be very unpleasantly shocked by), your use of untagged template strings makes it fundamentally impossible to mitigate such stuff, short of forbidding characters like & and < and " from data, which would be insanity. You're mixing template and data willy-nilly.

rglover 2y ago
The source isn't altered (I take the component HTML, parse it, build it into a DOM, and render it). The data from the API is compiled separately (where sanitization can happen if you have untrusted user data). The two are combined together during SSR by passing the sanitized data to the component. Developer controlled code at the component-level is assumed trusted, however, there are three mechanisms for handling escaping/sanitizing on the server: 1. There are helpers built in to the API (you can conditionally enable sanitization depending on need) to handle sanitization and helper functions built-in to the server-side to do one-off escaping/sanitization for HTML. 2. Query params are sanitized automatically to prevent injection. 3. To prevent unescaped HTML in data from breaking renders, I base64 encode it before sending it to the browser. This approach to escaping is by design as some use cases call for sanitization (they're rendering user-generated data) while others do not. You're welcome to audit the source and see if I missed something or make suggestions on improvements.
- chrismorgan 2y ago
  I wrote something much longer and more detailed, but I haven’t the heart to post it in the end. This is a phenomenally bad idea. By escaping things willy-nilly you’re making life much harder, and vastly more error-prone. You can only even vaguely sanely do such things with the support of a better type system than you have available to you. You’re encouraging storing escaped HTML in the database; exposing an API that HTML-encodes all strings; you’re misusing the term “sanitise” (sanitising is fairly consistently used to mean stripping control sequences, not escaping). This is a disaster. Given that you’re working in JavaScript, you need to use something like JSX or equivalent functions or tagged template strings, so you can escape only as appropriate. As it stands, I would genuinely expect to be able to find trivial functionality and/or security bugs in practically every decent-sized Joystick site involving user input. One specific technical bug: https://github.com/cheatcode/joystick/blob/canary/node/src/lib/escape_html.js https://github.com/cheatcode/joystick/blob/canary/node/src/l... tries to escape &<>"'`= but will replace backticks and equals signs with “undefined”. Defining HTML_ENTITY_MAP in a separate file was a mistake; you’ll hopefully never refer to it anywhere else, and separating it made such an error easier to make. (Incidentally, backtick and equals sign have never been useful things to escape in HTML. Truthfully, & and < are all you need in HTML text, and & and " in double-quoted attribute value. If you want to support XML, then escape < and > everywhere as well. But nothing beyond that is useful.)
  - rglover 2y ago
    I updated the escape_html() method to cover the missing characters/cases and centralized the entity map to the function definition. Appreciate you pointing that out. I went beyond what you suggested in terms of characters which should cover most (if not all) problem cases. Here's an example repo: https://github.com/cheatcode/escape_test https://github.com/cheatcode/escape_test And example text I used to test: https://gist.github.com/rglover/c75e37b526d864313b461d38195b6970 https://gist.github.com/rglover/c75e37b526d864313b461d38195b... > You’re encouraging storing escaped HTML in the database You can store unescaped HTML in the database and upon retrieval, escape it (see the linked demo repo above). You hypothetically could store escaped HTML, but you'd have to show me where I'm encouraging it (I think what you're saying is that because you can do it, I'm encouraging it). > As it stands, I would genuinely expect to be able to find trivial functionality and/or security bugs in practically every decent-sized Joystick site involving user input. You're welcome to audit it, but please, actually test it by building something and then provide an example repo. Make sure to use the canary release as that has the latest changes. If you email me (ryan.glover@cheatcode.co) I can give specifics on how to get set up. --- FWIW, this isn't a productive way to point out issues with a project. A PR with comments is preferred.
    - chrismorgan 2y ago
      Sorry, but it’s like the reaction to https://news.ycombinator.com/item?id=42359067 https://news.ycombinator.com/item?id=42359067: “this is neither a new idea nor a good one”. Ad-hoc escaping for serialisation is just fundamentally a really bad approach, and although it used to be how almost everything did it, nothing serious has tried working that way for at least a decade, probably closer to two by this point, because as an industry we’ve actually learned things. Your particular angle on it might be novel, I’m not certain, but only because it’s an even worse idea. Formerly, there was a simple rule: remember to escape data when emitting HTML. You’ve replaced that with something complex: some things will be entity-encoded, and others won’t; and often you won’t want things entity-encoded. (You’re inappropriately tying API to HTML, by the looks of it.) This is just a bad abstraction that makes errors sure, and errors will lead to some mangled text in all cases, and security bugs in some cases. It’s very similar to how the consensus is now well-established (though it’ll take plenty more time to be fully applied) in memory safety, a related security field: avoid languages like C, they’re too dangerous, use memory-safe languages instead. Another well-established principle: parse at input, let your system deal with data, and serialise at output. Your approach instead serialises at input—and that only most of the time—and hopes you never need to parse. I’m certain that I’ve upset you, and I’m sorry about that, but the approach really, truly is that bad, and I did just want to appeal to you to think it over. Look, if you can find someone experienced in web security and frontend framework design and things like that, preferably who’s been doing this for fifteen or twenty years, present the whole design to them, and see what they say. I genuinely expect horror. As for your changes to escape_html, I’ll just ask why? because you’ve compounded the error. There is no plausible purpose for all the additions you’ve made. By encoding, you declare intent to feed the value to an HTML or XML parser in data state, single-quoted attribute state or double-quoted attribute state. In none of those states do any of these additions achieve anything. And if you’re making them for some other purpose, then you don’t want the HTML escaping in the first place. Fifteen years ago, I would encounter both double-escaped HTML and entity-encoding in non-HTML contexts, far too often. Now, I feel like it’s years since I’ve seen such a thing, outside of text/plain parts of emails (but developers and marketers are frequently awful about text/plain parts). The only advantage I can see in entity-encoding more characters, especially all normal punctuation, is that bad escaping is going to be caught earlier. But I say, if you reckon those worth escaping, why not go the whole hog and .replace(/./ug, s => '&#' + s.codePointAt(0) + ';')? I’m not sure what your experience is, but the impression I’m getting is that you’ve been largely just JavaScript. A lot of the problems you’re producing were best-known through PHP, and better type systems are a large part of the best solutions to these things. If I’m sounding you out correctly, I’d recommend that you look into and learn some strong statically-typed language, preferably one with algebraic data types (also called sum types). Rust has been my own preference for the last decade, but there are plenty of others that would guide you in similar lessons, like (in no particular order) C♯, Swift, Elixir, Kotlin, Scala, Haskell. (I don’t include TypeScript in this list, although it could definitely help with some of the lessons, because it’s too compromised by JavaScript limitations.) If you haven’t had any of this sort of experience, getting it will help you to write better JavaScript, and make better systems. —⁂— Now, for a specific matter: > You hypothetically could store escaped HTML, but you'd have to show me where I'm encouraging it <https://docs.cheatcode.co/joystick/node/escape_html#:~:text=The%20escape_html()%20method%20is%20helpful%20for%20pre-sanitizing%20user%20input%20before%20it%20hits%20your%20database. https://docs.cheatcode.co/joystick/node/escape_html#:~:text=...> (And if you unconditionally entity-encode query string parameters… I don’t even want to think about the implications of that.) I would also point out that https://docs.cheatcode.co/joystick/node/app/api/sanitization https://docs.cheatcode.co/joystick/node/app/api/sanitization says: > Sanitization is the process of scrubbing unexpected or unwanted HTML tags from the values returned by your API. But this is not what you’ve implemented. You don’t actually scrub, you entity-encode.