tl;dr Raw markup injection (e.g. through innerHTML) in system-privileged content is automatically sanitized and we have removed all exceptions to this rule. You should still not use innerHTML and friends.
Hello everyone, The danger of remote code execution (RCE) exploits through unescaped innerHTML usage was recently brought to renewed public attention through the finding of a spectacular vulnerability in the Signal Desktop app <https://thehackerblog.com/i-too-like-to-live-dangerously-accidentally-finding-rce-in-signal-desktop-via-html-injection-in-quoted-replies/index.html>. As most of you know, the Firefox front-end, like Signal, is written using web technologies, which means that innerHTML and similar techniques of injecting raw markup are a constant threat to us as well. An injection vulnerability in system privileged desktop UI can safely be considered a worst-case scenario. Once found, it is usually very easy to exploit and grants the attacker full system privileges without requiring a sandbox escape. The Firefox Security Engineering team identified this threat some time ago and we have been making slow but constant progress towards a solution. Today, I'm happy to announce that there is not a single instance of potentially unsafe innerHTML usage in system-privileged Firefox code. How can I be sure? We started sanitizing system privileged innerHTML assignments at the platform level <https://bugzilla.mozilla.org/show_bug.cgi?id=1432966>. The Sanitizer will drop (remove) any nodes or attributes from the supplied markup that are considered unsafe. (See this code <https://searchfox.org/mozilla-central/rev/83a923ef7a3b95a516f240a6810c20664b1e0ac9/dom/base/nsContentUtils.cpp#5086> and that code <https://searchfox.org/mozilla-central/rev/83a923ef7a3b95a516f240a6810c20664b1e0ac9/dom/base/nsTreeSanitizer.h#48>) It will also log a warning about it. The initial patch had enough breakage that it forced us to add an exception to this rule, in the form of a function called unsafeSetInnerHTML, which could be called in place of innerHTML assignment. During the last months we have been working on removing this function again, fixing up the individual callers with safe alternatives. The devtools team recently (heroically) removed the GCLI UI (and is working to replace some of its most popular features) to support this effort. Yesterday we landed patches to remove unsafeSetInnerHTML <https://bugzilla.mozilla.org/show_bug.cgi?id=1444394> again, leaving no way to bypass sanitization in chrome code. Note that, for obvious reasons, we will not be able to have sanitization in regular content-privileged contexts. This is one reason why we still run an ESLint plugin called eslint-plugin-no-unsanitized <https://github.com/mozilla/eslint-plugin-no-unsanitized> that will call out any potentially unsafe usage of innerHTML. Please be extra cautious around old code that is highlighted with an exception to that lint and please, do not introduce new exceptions. So, can we use more innerHTML now? NO. Please do not use innerHTML, even for the simplest use cases. The sanitization only prevents the worst category of attacks and user input could still mess with the display or even behavior of your UI. It's also not very performant and you are running the risk of eventually moving it into content-privileged code, which is not sanitized. What if I know how to sanitize this stuff? Experience tells us that while most developers understand that "node.innerHTML = userProvidedData" is bad, that's not necessarily how these vulnerabilities get introduced. The more your code gets moved or refactored the higher the risk of making it vulnerable along the way, even if your originally added innerHTML in a safe manner. We have seen vulnerabilities which were introduced through a series of refactorings, none of which actually touched the eventual innerHTML assignment. Don't roll your own innerHTML sanitization, please. Someone else will break it. Thank you to all the great people who made this possible (in no particular order): Boris Zbarsky, Mike Conley, J. Ryan Stinnet, Andrew Swan, Gijs Kruitbosch, Prathiksha, Trisha Gupta, Jorg K, Mark Striemer, Dan Veditz, Nihanth Subramanya, Wennie Leung And a special thanks to: Frederik Braun, for being the author of eslint-plugin-no-unsanitized (and first to recognize this problem) and for all his general counsel. Kris Maglione, for coming up with the original sanitization plan. Joe Walker, Patrick Brosset, Soledad Penades and Alexandre Poirot, for removing GCLI. Please let me know if you have any other questions or concerns. Keep on rocking the free <img src="" onerror="alert('Web')" />, Johann _______________________________________________ dev-platform mailing list [email protected] https://lists.mozilla.org/listinfo/dev-platform
