fix(console): message formatting XSS vulnerability#2247
Conversation
ff18ae9 to
61b90a0
Compare
Having HTML characters like `<` in a message will currently break it's
rendering. An example would be a standard Python object `__repr__`,
like it might show up in an error message:
<klippy.extras.gcode_macro.GetStatusWrapperPython object at 0x7f67eb2cd0>
This is an XSS vulnerability. By making Klippy `M118` a message
containing HTML, you could hijack the browser of everyone currently
monitoring the console.
But even if you consider this an unlikely attack vector, this will
still fix quirky visual behavior in current message rendering, though.
61b90a0 to
8412813
Compare
|
@jwueller Thank you very much! Pls also add an option in the Interface Settings to disable this feature/fix? Some projects like AFC-Klipper-Add-On or HappyHare use this for "HTML-Outputs". I would enable the escape per default, but users which use plugins like these, can disable it for "better format html outputs". |
|
Closed, because i got no answer... |
|
@meteyou My apologies, I must have missed this among my flurry of notifications. I suppose this could be toggled, but the core issue is that we have no way to be certain about the intended message format. Either setting would produce broken output in some circumstances. The real fix would be a native format that's attached to an individual message, so it can be rendered appropriately. However, that would likely require support from Klipper, as well as coordination with other frontends. Since the PR no longer seems to be feasible as-is, my proposal would be to do HTML sanitization instead, to strip out any unrecognized tags (e.g. https://github.com/cure53/DOMPurify). This would allow authors to keep using common rich text elements like text styles or links, but also significantly reduce the potential attack surface. If you're happy with the proposal, feel free to reopen this and I'll make the required changes. |
|
Addendum: I just realized that Klipper already has a message type prefix, so maybe we should encourage standardizing around something like this? RESPOND TYPE=echo_html MSG="<message>" # will likely error due to unknown value in Klipper, but:
RESPOND PREFIX="echo_html:" MSG="<message>" # takes precedence over TYPE
RESPOND PREFIX="echo:html:" MSG="<message>" # better backwards compatibilityAnd then consider regular Although I should add, that even in this case, HTML sanitization should still happen. |
|
Hi @jwueller, thanks for your interest in contributing to Mainsail! 👋 This pull request has been automatically closed because pull requests may only be opened by vouched contributors, and you are not yet on our vouched list. This is not a rejection of your work. A maintainer can vouch for you and once that happens, simply reopen this PR or comment Please see our contributing guidelines for more details: |
|
vouch @jwueller |
Description
Having HTML characters like
<in a message will currently break it's rendering. An example would be a standard Python object__repr__, like it might show up in an error message:This is an XSS vulnerability. Making Klippy echo a message containing HTML could run arbitrary code in the browser of everyone currently monitoring the console.
But even if you consider this an unlikely attack vector, this will still fix quirky visual behavior in current message rendering, though.
Related Tickets & Documents
None as far as I can tell, but I can create one for this vulnerability, if required.