5 ms·

The downside of a new method is that it leaves innerHtml as a source of future security issues.

by philipwhiuk 11mo ago

cluckindan 11mo ago
Yes, one could simply make a setter for innerHTML which calls setHTML(). No code changes needed.
- masklinn 11mo ago
  That breaks existing usages of innerhtml which may legitimately need its more dangerous features.
  - cxr 11mo ago
    It seems obvious enough that parent is talking about changing the behavior of innerHTML within their own application, not for browser makers to change the implementation. It's unfair to take the most uncharitable interpretation and upbraid the other commenter for being insufficiently defensive[1] when they pressed "reply". 1. <https://pchiusano.github.io/2014-10-11/defensive-writing.html https://pchiusano.github.io/2014-10-11/defensive-writing.htm...>
    - masklinn 11mo ago
      > It seems obvious Doesn't seem obvious unless your dutch. Especially as the first things I would think obvious is: if breaking the behaviour of innerHTML is not a concern for your software why keep it at all? Delete the property or make it readonly.
      - cxr 11mo ago
        > Doesn't seem obvious unless your dutch. I don't know what that means. > if breaking the behaviour of innerHTML is not a concern for your software why keep it at all? For the reason that they said.
        shadowgovt 11mo ago
        > Doesn't seem obvious unless your dutch. I believe this is a reference to the old "Zen of Python," which one gets by running `import this` in the Python REPL. """ There should be one-- and preferably only one --obvious way to do it. Although that way may not be obvious at first unless you're Dutch. """
  - cluckindan 11mo ago
    Maybe I should have included the word ”monkeypatch” in the comment.
crote 11mo ago
Yes, but you can also easily lint on it: all uses of `context.innerHTML` are now suspect and should get a suggestion to use `context.setHTML` instead. With `const clean = DOMPurify.sanitize(input); context.innerHTML = clean;` your linter suddenly needs to do complex code analysis and keep track if each variable passed to `context.innerHTML` is clean or tainted.
wbobeirne 11mo ago
I feel like calling this a downside implies there's an alternative, but there's no way that `innerHtml`'s behavior could be changed. There are a lot of valid reasons for arbitrary HTML to be set, and changing that would break so many things.
- cortesoft 11mo ago
  There could be a better name for it? like `innerSanitizedHTML` or something, that makes it clear what the difference between the two calls are. There is nothing in the wording of setHTML that makes it clear it sanitizes where innerHTML doesn't.
uallo 11mo ago
Combine it with enforced Trusted Types: https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Types_API https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Typ...
rictic 11mo ago
You can disable it for your site using a trusted types content security policy.

The downside of a new method is that it leaves innerHtml as a source of future security issues.

cluckindan 11mo ago

Yes, one could simply make a setter for innerHTML which calls setHTML(). No code changes needed.

masklinn 11mo ago
That breaks existing usages of innerhtml which may legitimately need its more dangerous features.
- cxr 11mo ago
  It seems obvious enough that parent is talking about changing the behavior of innerHTML within their own application, not for browser makers to change the implementation. It's unfair to take the most uncharitable interpretation and upbraid the other commenter for being insufficiently defensive[1] when they pressed "reply". 1. <https://pchiusano.github.io/2014-10-11/defensive-writing.html https://pchiusano.github.io/2014-10-11/defensive-writing.htm...>
  - masklinn 11mo ago
    > It seems obvious Doesn't seem obvious unless your dutch. Especially as the first things I would think obvious is: if breaking the behaviour of innerHTML is not a concern for your software why keep it at all? Delete the property or make it readonly.
    - cxr 11mo ago
      > Doesn't seem obvious unless your dutch. I don't know what that means. > if breaking the behaviour of innerHTML is not a concern for your software why keep it at all? For the reason that they said.
      - shadowgovt 11mo ago
        > Doesn't seem obvious unless your dutch. I believe this is a reference to the old "Zen of Python," which one gets by running `import this` in the Python REPL. """ There should be one-- and preferably only one --obvious way to do it. Although that way may not be obvious at first unless you're Dutch. """
- cluckindan 11mo ago
  Maybe I should have included the word ”monkeypatch” in the comment.

crote 11mo ago

Yes, but you can also easily lint on it: all uses of `context.innerHTML` are now suspect and should get a suggestion to use `context.setHTML` instead. With `const clean = DOMPurify.sanitize(input); context.innerHTML = clean;` your linter suddenly needs to do complex code analysis and keep track if each variable passed to `context.innerHTML` is clean or tainted.

wbobeirne 11mo ago

I feel like calling this a downside implies there's an alternative, but there's no way that `innerHtml`'s behavior could be changed. There are a lot of valid reasons for arbitrary HTML to be set, and changing that would break so many things.

cortesoft 11mo ago
There could be a better name for it? like `innerSanitizedHTML` or something, that makes it clear what the difference between the two calls are. There is nothing in the wording of setHTML that makes it clear it sanitizes where innerHTML doesn't.

uallo 11mo ago

Combine it with enforced Trusted Types: https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Types_API https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Typ...

rictic 11mo ago

You can disable it for your site using a trusted types content security policy.