10 comments

  • entuno 2 hours ago
    This kind of thing always makes me nervous, because you end with a mix of methods where you can (supposedly) pass arbitrary user input to them and they'll safely handle it, and methods where you can't do that without introducing vulnerabilities - but it's not at all clear which is which from the names. Ideally you design that in from the state, so any dangerous functions are very clearly dangerous from the name. But you can't easily do that down the line.

    I'm also rather sceptical of things that "sanitise" HTML, both because there's a long history of them having holes, and because it's not immediately clear what that means, and what exactly is considered "safe".

    • Cthulhu_ 2 minutes ago
      Ideally you should be able to set a global property somewhere (as a web developer) that disallows outdated APIs like `innerHTML`, but with the Big Caveat that your website will not work on browsers older than X. But maybe there's web standards for that already, backup content if a browser is considered outdated.
    • jncraton 1 hour ago
      You are right that the concept of "safe" is nebulous, but the goal here is specifically to be XSS-safe [1]. Elements or properties that could allow scripts to execute are removed. This functionality lives in the user agent and prevents adding unsafe elements to the DOM itself, so it should be easier to get correct than a string-to-string sanitizer. The logic of "is the element currently being added to the DOM a <script>" is fundamentally easier to get right than "does this HTML string include a script tag".

      [1] https://developer.mozilla.org/en-US/docs/Web/API/Element/set...

    • voxic11 1 hour ago
      The idea is you wouldn't mix innerHTML and setHTML, you would eliminate all usage of innerHTML and use the new setHTMLUnsafe if you needed the old functionality.
      • extraduder_ire 48 minutes ago
        I looked up setHTMLUnsafe on MDN, and it looks like its been in every notable browser since last year.

        Good idea to ship that one first, when it's easier to implement and is going to be the unsafe fallback going forward.

      • croes 1 hour ago
        If I need the old functionality why not stick to innerHTML?
        • orf 1 hour ago
          because the "unsafe" suffix conveys information to the reader, whereas `innherHTML` does not?
          • goatlover 1 minute ago
            Any potential reader should be familiar with innerHTML.
        • tbrownaw 51 minutes ago
          Because then your linter won't be able to tell you when you're done migrating the calls that can be migrated.
      • reddalo 59 minutes ago
        You can't rename an existing method. It would break compatibility with existing websites.
      • post-it 1 hour ago
        > you would eliminate all usage of innerHTML

        The mythical refactor where all deprecated code is replaced with modern code. I'm not sure it has ever happened.

        I don't have an alternative of course, adding new methods while keeping the old ones is the only way to edit an append-only standard like the web.

        • thenewnewguy 1 hour ago
          If you want to adopt this in your project, you can add a linter that explicitly bans innerHTML (and then go fix the issues it finds). Obviously Mozilla cannot magically fix the code of every website on the web but the tools exist for _your_ website.
        • littlestymaar 1 minute ago
          Nobody's talking about old code here.

          Having an alternative to innerHTML means you can ban it from new code through linting.

        • Vinnl 1 hour ago
          I kinda like the way JS evolved into a modern language, where essentially ~everyone uses a linter that e.g. prevents the use of `var`. Sure, it's technically still in the language, but it's almost never used anymore.

          (Assuming transpilers have stopped outputting it, which I'm not confident about.)

          • delaminator 1 hour ago
            for some values of "everyone" and "never".
          • thunderfork 1 hour ago
            Depending on the transpiler and mode of operation, `var` is sometimes emitted.

            For example, esbuild will emit var when targeting ESM, for performance and minification reasons. Because ESM has its own inherent scope barrier, this is fine, but it won't apply the same optimizations when targeting (e.g.) IIFE, because it's not fine in that context.

            https://github.com/evanw/esbuild/issues/1301

        • bulbar 1 hour ago
          It for sure happens for drop in replacements.
        • noduerme 1 hour ago
          Finally, a good use case for AI.
          • Aachen 1 hour ago
            Yeah, using a kilowatt GPU for string replacement is going to be the killer feature. I probably shouldn't even be joking, people are using it like this already
            • littlestymaar 0 minutes ago
              This ship has sailed unfortunately, no later than yesterday I've seen coworkers redact a screenshot using chatGTP.
            • charcircuit 1 hour ago
              When the condition for when you want to replace is hard to properly specify, AI shines for such find and replaces.
              • Aachen 35 minutes ago
                This one is literally matching "innerHTML = X" and setting "setHTML(X)" instead. Not some complex data format transformation

                But I can see what you mean, even if then it would still be better for it to print the code that does what you want (uses a few Wh) than doing the actual transformation itself (prone to mistakes, injection attacks, and uses however many tokens your input data is)

          • josefx 1 hour ago
            Wouldn't AI be trained on data using innerHTML?
            • Aachen 1 hour ago
              My experience is that they somehow print quite modern code despite things like ES6 being too new to be standard knowledge even for me and I'm not even middle-aged yet

              Maybe the last 10 years saw so much more modern code than the last cumulative 40+ years of coding and so modern code is statistically more likely to be output? Or maybe they assign higher weights to more recent commits/sources during training? Not sure but it seems to be good at picking this up. And you can always feed the info into its context window until then

              • skeeter2020 24 minutes ago
                This is not my experience. Claude has been happily generating code over the past week that is full of implicit any and using code that's been deprecated for at least 2 years.

                >> Maybe the last 10 years saw so much more modern code than the last cumulative 40+ years of coding and so modern code is statistically more likely to be output?

                The rate of change has made defining "modern" even more difficult and the timeframe brief, plus all that new code is based on old code, so it's more like a leaning tower than some sort of solid foundation.

            • charcircuit 1 hour ago
              Which is why it can easily understand how innerHTML is being used so that it can replace it with the right thing.
            • stvltvs 1 hour ago
              Honest question: Is there a way to get an LLM to stop emitting deprecated code?
              • fragmede 1 hour ago
                Theoretically, if you could train your own, and remove all references to the deprecated code in the training data, it wouldn't be able to emit deprecated code. Realistically that ability is out of reach at the hobbiest level so it will have to remain theoretical for at least a few more iterations of Moore's law.
    • snowhale 1 hour ago
      the browser-native Sanitizer API has one advantage the library approaches don't: it uses the same HTML parser the browser uses to render. libraries like DOMPurify parse in a separate context then re-serialize, and historically that round-trip is where most bypasses came from. when the sanitizer and the renderer share the same parser, mutation XSS attacks have nowhere to hide.
      • pornel 47 minutes ago
        BTW, HTML allows inline SVG with an XML-flavored syntax that interprets <script/> and <title> differently. It's a goldmine for sanitizer escapes. There are completely bonkers syntax switching and error recovery rules that interact with parsing modes (there's even an edge case where a particular attribute value switches between HTML and XML-ish parsing rules).

        Don't even try to allow inline <svg> from untrusted sources! (and then you still must sanitise any svg files you host)

    • DoctorOW 1 hour ago
      They do link the default configuration for "safe": https://wicg.github.io/sanitizer-api/#built-in-safe-default-...

      But I agree, my default approach has usually been to only use innerText if it has untrusted content:

      So if their demo is this:

          container.SetHTML(`<h1>Hello, {name}</h1>`);
      
      Mine would be:

          let greetingHeader = container.CreateElement("h1");
          greetingHeader.innerText = `Hello, {name}`;
      • itishappy 42 minutes ago
        What if I wanted an <h2>?
    • jaffathecake 45 minutes ago
      fwiw, if you serve your page with:

      Content-Security-Policy: require-trusted-types-for 'script'

      …then it blocks you from passing regular strings to the methods that don't sanitize.

    • noduerme 1 hour ago
      Some sanitization is better than none? If you're relying on the browser to handle it for you, you're already in a lot of trouble.
    • post-it 1 hour ago
      realSetSafeHTML()
  • Aachen 1 hour ago
    So you can still inject <h1> or <br><br><br>... etc into your username, in the given example

    Preventing one bug class (script execution) is good, but this still allows arbitrary markup to the page (even <style> CSS rules) if I'm reading the docs correctly. You could give Paypal a fresh look for anyone who opens your profile page, if they use this. Who would ever want this?

    • piccirello 6 minutes ago
      `setHTML` is meant as a replacement for `innerHTML`. In the use case you describe, you would have never wanted `innerHTML` anyway. You'd want `innerText` or `textContent`.
    • jerf 23 minutes ago
      If I'm reading this right,

          .setHTML("<h1>Hello</h1>", new Sanitizer({}))
      
      will strip all elements out. That's not too difficult.

      Plus this is defense-in-depth. Backends will still need to sanitize usernames on some standard anyhow (there's not a lot of systems out there that should take arbitrary Unicode input as usernames), and backends SHOULD (in the RFC sense [1]) still HTML-escape anything they output that they don't want to be raw HTML.

      [1]: https://www.rfc-editor.org/rfc/rfc2119

      • benmmurphy 2 minutes ago
        i think the use case for setHTML is for user content that contains rich text and to display that safely. so this is not an alternative for escaping text or inserting text into the DOM but rather a method for displaying rich text. for example maybe you have an editor that produces em, and strong tags so now you can just whitelist those tags and use setHTML to safely put that rich text into the DOM without worrying about all the possible HTML parsing edge cases.
    • itishappy 37 minutes ago
      > If the default configuration of setHTML( ) is too strict (or not strict enough) for a given use case, developers can provide a custom configuration that defines which HTML elements and attributes should be kept or removed.
      • Aachen 32 minutes ago
        Injecting markup into someone else's website isn't what I'd call too strict a default configuration

        If you mean to convey that it's possible to configure it to filter properly, let me introduce you to `textContent` which is older than Firefox (I'm struggling to find a date it's so old)

        • itishappy 28 minutes ago
          That's the whole point of the setHTML.

          How would I set a header level using textContent?

          • Aachen 19 minutes ago
            The traditional way: separating data and code

                document.createElement("h1").textContent = `Hello, ${username}!`
            
            If you allow <h1> in the setHTML configuration or use the default, users with the tag in their username also always get it rendered as markup
            • matsemann 12 minutes ago
              Which is why you only use it where you want to allow some kind of html..?
    • cogman10 1 hour ago
      > Who would ever want this?

      The main case I can think of is wanting some forum functionality. Perhaps you want to allow your users to be able to write in markdown. This would provide an extra layer of protection as you could take the HTML generated from the markdown and further lock it down to only an allowed set of elements like `h1`. Just in case someone tried some of the markdown escape hatches that you didn't expect.

      • Aachen 59 minutes ago
        > This would provide an extra layer of protection

        I think this might be the answer. There's no point to it by itself (either you separate data and code or you don't and let the user do anything to your page), but if you're already using a sanitiser and you can't use `textContent` because (such as with Markdown) there'll be HTML tags in the output, then this could be extra hardening. Thanks!

    • byproxy 58 minutes ago
      > but this still allows arbitrary markup to the page (even <style> CSS rules) if I'm reading the docs correctly.

      If that's true, seems like it's still a security risk given what you can do with CSS these days: https://news.ycombinator.com/item?id=47132102

      • circuit10 11 minutes ago
        You can use selectors to gain some information about things like input fields, e.g. https://www.invicti.com/blog/web-security/private-data-stole...

        Or I guess you could completely restyle and change the text of UI elements so it looks like the user is doing one thing when they're actually doing something completely different like sending you money

    • embedding-shape 1 hour ago
      > So you can still inject <h1> or <br><br><br>... etc into your username, in the given example

      How exactly, given that setHTML sanitizes the input? If you don't want to have any HTML tags allowed, seems you can configure that already? https://wicg.github.io/sanitizer-api/#built-in-safe-default-...

      • Aachen 1 hour ago
        > How exactly, given that setHTML sanitizes the input?

        The article says that the output is:

            <h1>Hello my name is</h1>
        
        So it keeps (non-script) html tags (and presumably also attributes) in the input. Idk how you're asking "how" since it's the default behavior

        Stripping HTML tags completely has always been possible with the drop-in replacement `textContent`. Making a custom configuration object for that is much more roundabout

        • embedding-shape 1 hour ago
          Yes, because that's the default configuration, if you don't want that, stop using the default configuration? It's still sanitizing away the common XSS holes, hence it's a safer alternative to .innerHTML, and a more flexible alternative to .innerText
          • Aachen 51 minutes ago
            Shouldn't use innerText anyway (nonstandard, worse performance, tries to parse the HTML and gives you unexpected behavior if e.g. a style is set that makes an element invisible but still has text inside, doesn't work on all DOM nodes...)

            I can see how it's a way of allowing some tags like bold and italic without needing a library or some custom parser, but I didn't understand what the point of this default could be and so why it exists (a sibling comment proposed a plausible answer: hardening on top of another solution)

            > Yes, because that's the default configuration, if you don't want that, stop using the default configuration?

            "don't use it if it's not what you want" is perhaps the silliest possible answer to the question "what's the use-case for this"

            • embedding-shape 45 minutes ago
              > Shouldn't use innerText anyway (nonstandard, worse performance, tries to parse the HTML and gives you unexpected behavior if e.g. a style is set that makes an element invisible but still has text inside, doesn't work on all DOM nodes...)

              Maybe you meant .innerHTML? .innerText AFAIK doesn't try to parse HTML (why would it?), but I don't understand what you mean with nonstandard, both .innerHTML and .innerText are part of the standards, and I think they've been for a long time.

              > but I didn't understand what the point of this default could be and so why it exists (a sibling comment proposed a plausible answer: hardening on top of another solution) [...] the question "what's the use-case for this"

              I guess maybe third time could be the charm: it's for preventing XSS holes that are very common when people use .innerHTML

              • Aachen 24 minutes ago
                > maybe third time could be the charm: it's for preventing XSS holes

                That information is in the question, so sadly no this still doesn't make sense to me because I don't understand any scenario in which this is what the developer wants. You always still need more code (to filter the right tags) or can just use textContent (separating data and code completely, imo the recommended solution)

                > Maybe you meant .innerHTML? .innerText AFAIK doesn't try to parse HTML (why would it?)

                No, I didn't mean that, yes it does, and no I don't know why it is this way. If you don't believe me and don't want to check it out for yourself, I'm not sure what more I can say to help the conversation

            • benregenspan 41 minutes ago
              It seems like the goal of the default configuration is preventing script injection while being otherwise very permissive. Basically, "safer than innerHTML, even when used very lazily". But I would expect guidance to evolve saying that it almost never makes sense to use the default and instead to specify a configuration that makes contextual sense for a given field.

              The default might be suitable for something like an internal blog where you want to allow people to sometimes go crazy with `<style>` tags etc, just not inject scripts, but I would expect it to almost always make sense to define a specific allowed tag and attribute list, as is usually done with the userland predecessors to this API.

  • simonw 2 hours ago
    Great to see this start to show up, but it looks like it will be a while before browser support is widely distributed enough to rely on it being present: https://caniuse.com/mdn-api_element_sethtml
    • jraph 1 hour ago
      Indeed, as any browser API, it might be for in a few years (months if happy with the most recent versions), and we may have polyfills in the meantime.
      • tuyiown 1 hour ago
        I wouldn't advise polyfills on this one, it entirely depends on the browser ability to evaluate cross scripting and cross origin rule on a arbitrary snippet. This is not a convenience API.
  • cogman10 1 hour ago
    Seems like this has a bunch of footguns. Particularly if you interact with the Sanitizer api, and particularly if you use the "remove" sanitizer api.

    Don't get me wrong, better than nothing, but also really really consider just using "setText" instead and never allow the user to add any sort of HTML too the document.

    • evilpie 52 minutes ago
      Using an allowlist based Sanitizer you are definitely less likely to shoot yourself in the foot, but as long as you use setHTML you can't introduce XSS at least.
  • tuyiown 1 hour ago
    This is nice. The best part is that all aspects of network access are now properly controlled so that security transitioned from a chain of trusted code to a chain of trusted security setup on hosts, with existing workable safe defaults.
  • shadowgovt 9 minutes ago
    Oh, that's nice-to-have. Good work, Mozilla.

    It would close the loop better if you could also use policy to switch off innerHTML in a given page, but definitely a step in the right direction for plain-JavaScript applications.

  • dbvn 55 minutes ago
    at what point can we consider the development of "set this element's text/html" to be done?
    • Aachen 42 minutes ago
      When browsers implement a variant that lets you separate data and code perhaps. That's what I expected when reading the headline: setHtml(code, data, data, ...), just like parameterised SQL works: prepare("select rowid from %s where time < %n", tablename, mynumber)

      This new method they've cooked up would be called eval(code,options) if html was anything other than a markup language

  • bingemaker 1 hour ago
    Nice one. Will there be any impact on __dangerouslySetInnerHTML (React)?
  • antonyh 1 hour ago
    A rather deceptive title, given that 'innerHTML' isn't going away.
  • octoclaw 1 hour ago
    [dead]