9 ms·

Maybe secure chat clients shouldn't be written in JavaScript or other languages that have excessive dynamicness? Signal seems to be written mostly in languages

by tormeh 8y ago

Maybe secure chat clients shouldn't be written in JavaScript or other languages that have excessive dynamicness? Signal seems to be written mostly in languages that are bad for security (significantly worse than the best alternatives). Maybe I'm just a language nerd without any clue about the trade-offs, but I trust the Wire software more. Note that this just applies to mobile clients and server - Wire, like Signal, chose to build their desktop+webapp in JavaScript :(

pfg 8y ago
As much as I'm not a fan of JavaScript, the problem is not so much the language but rather the choice of Electron and all that comes with it. Heck, even a web version or Chrome app would've successfully mitigated these attacks. Electron means you're one XSS away from remote code execution, and even worse, it makes it way harder to mitigate XSS through CSP (which Signal did utilize, but script-src 'self' can easily be bypassed in Electron). FWIW, Signal's native mobile apps are written in Java and Objective-C respectively, so there's not really much of a difference compared to Wire (which is a good choice as well). Still, even a hypothetical React Native app written in JavaScript wouldn't be much worse; after all, React Native isn't just a Web View made to look like a native app, but uses actual native components.
- mintplant 8y ago
  Signal Desktop actually used to be a Chrome app. Then Google announced the deprecation of that feature and they ported it over to Electron.
  - ehPReth 8y ago
    Sad day for Chrome OS users. No more Signal updates! :(
- ams6110 8y ago
  > Electron means you're one XSS away from remote code execution So, electron is the new flash. I'll be avoiding that, then.
  - deleted 8y ago
    [deleted]
- rndgermandude 8y ago
  While I agree that Electon offers a massive amount of footguns, neither Javascript nor Electon was the issue in this case. The issue was using innerHTML (or rather $.html()) with strings concatenated together from user input. Something you should never do. Could as well just call eval() directly on it, or pass the input to gcc, compile it and run the resulting binary. The Signal devs thought $.html() does some kind of escaping: https://github.com/signalapp/Signal-Desktop/commit/9d41b8616296f1b328aa864e0114b99d7f11ca06 https://github.com/signalapp/Signal-Desktop/commit/9d41b8616... (this commit made something that was easy to exploit into something that was even easier to exploit). To be honest, I'd lay more blame on the authors of the DOM spec making innerHTML a setter than on Electron, and jQuery exposing this misfeature even more with $.html(), teaching an army of web developers to do the wrong thing. We've all seen numerous (XSS) vulnerabilities in all kinds of websites, browser extensions, Electron apps, etc resulting from this API, tho in Electron apps it gets particularly devastating as often you'd get code execution not just in a sandboxed website but full code execution under the current user credentials in the system.
  - pfg 8y ago
    So to be clear, a lot of the blame definitely belongs in the "all that comes with it" bucket here, which is one of the reasons why you should think twice about developing desktop apps using a platform that forces you to deal with not only the usual desktop app security concerns, but also all the things that make web apps vulnerable. Still, when you ship an app with a relatively strict Content Security Policy as Signal did (including using script-src 'self'), you don't really expect a simple XSS vulnerability to lead to RCE, but it turns out that policy doesn't really do much in an Electron app.
  - caf 8y ago
    Surely the collective noun for footguns is a cache :)
  - seba_dos1 8y ago
    >The Signal devs thought $.html() does some kind of escaping: Uhm... that's a really rookie mistake to make. Like, one of the very basics of jQuery usage. I'm not exactly sure what to think about it after seeing this commit you linked...
- AgentME 8y ago
  >Electron means you're one XSS away from remote code execution, and even worse, it makes it way harder to mitigate XSS through CSP (which Signal did utilize, but script-src 'self' can easily be bypassed in Electron). Can you explain that last part? I can't think of how an XSS attack could get around that, and Electron's documentation specifically recommends it: https://github.com/electron/electron/blob/master/docs/tutorial/security.md#6-define-a-content-security-policy https://github.com/electron/electron/blob/master/docs/tutori...
  - pfg 8y ago
    There's a bit of an explanation of this in the article describing the other XSS that's recently been found in Signal[1]. Basically, since the Electron app itself runs under the file:// origin, 'self' can be bypassed with varying degrees of difficulty depending on the platform. On Windows, it's trivial because you can use UNC paths to a SMB share containing a malicious JavaScript file (i.e. file://1.2.3.4/payload.js). On other platforms, you'd need to find a way to place the file on a path accessible via file:// first, for example by sending the file via Signal itself and hoping the user accepts the download. There are ways to lock down the CSP further to mitigate this, but no one really expects script-src 'self' to be unsafe, especially when it's what their documentation recommends. [1]: https://ivan.barreraoro.com.ar/signal-desktop-html-tag-injection/advisory/ https://ivan.barreraoro.com.ar/signal-desktop-html-tag-injec...
- burtonator 8y ago
  One thing I'd like to see is more use of containers and permissions locally. For example, my IntelliJ runs as my user account, but it doesn't need access to all my files. I should be able to select which directories it has access to and it's within a container by default. I mean I can set this stuff up manually, but in the future I'd like to see this as the default. Similar to the way Android apps ask for permissions.
  - kilburn 8y ago
    It seems you are calling for Qubes OS [1], which does that but using VMs (which should be more secure than containers). It will take a looong time for "standard" OSes to get there, if they ever do. The required changes in UX are very significant... [1] https://www.qubes-os.org/ https://www.qubes-os.org/
  - pjmlp 8y ago
    That is the whole idea of the UWP model on Windows, and the ongoing work to put Win32 apps inside of the same containers. Or the sandbox models on Android, iOS and macOS.
    - danieldk 8y ago
      And Flatpak on Linux. IIRC the Signal Flatpak is sandboxed.
- Rjevski 8y ago
  I disagree; the issue is Electron. Sure, you can write secure apps in Electron, just like you can do risky stuff in real-life and be fine most of the time. But why take the risk? Had the app been written with a native language and SDK, they wouldn’t need to worry about escaping or anything. I have yet to hear about getting remote code execution for dumping text into an UILabel or similar, while XSS happens almost every day.
symlinkk 8y ago
None of this is caused because of them using a dynamic language. It's caused because the developers used a function literally called "dangerouslySetInnerHTML" that doesn't escape HTML. That's it. It's just lazy programming.
- vjeux 8y ago
  We called it that way in React in order to call out attention to the fact that it was actually dangerous. React also properly escapes everything else it prints. The app isn't using React but jQuery, which doesn't have those protections.
  - deleted 8y ago
    [deleted]
  - archgoon 8y ago
    This doesn't seem to be true, here's the v1.10.0 code: https://github.com/signalapp/Signal-Desktop/blob/f6eb745632ce87eabce93d3c12f66f0bc6146d2d/ts/components/conversation/Quote.tsx#L114 https://github.com/signalapp/Signal-Desktop/blob/f6eb745632c... They do seem to be using react, and using dangerouslySetInnerHTML. Now that said, I haven't confirmed that this is the code that caused the issue, but it is in the Quotes component, which is referenced in the article. They seem to have fixed this specific issue a few days ago (v.11.0): https://github.com/signalapp/Signal-Desktop/blob/0d00fbfb7a23a9b05ea9e0c74910d31a27e710de/ts/components/conversation/Quote.tsx https://github.com/signalapp/Signal-Desktop/blob/0d00fbfb7a2... They do seem to be using jQuery elsewhere in the code base, but I'm not familiar enough to determine how it all fits together.
    - ddtaylor 8y ago
      They are using many frameworks mixed together like jQuery, React, and underscore.
    - vjeux 8y ago
      Oh sorry you are right! I was reading the comment ( https://news.ycombinator.com/item?id=17097501 https://news.ycombinator.com/item?id=17097501 ) which said that the issue was caused with $.html
connorelsea 8y ago
there is plenty of secure software written in javascript. poor engineering can occur in any language
- 4ad 8y ago
  Can you provide some examples?
  - kenforthewin 8y ago
    openpgp.js, professionally audited several times over https://openpgpjs.org/ https://openpgpjs.org/
    - na85 8y ago
      Audited by whom? Where are their findings?
      - connorelsea 8y ago
        Googled it for you. From their github repo... "To date the OpenPGP.js code base has undergone two complete security audits from Cure53. The first audit's report has been published here." https://github.com/openpgpjs/openpgpjs https://github.com/openpgpjs/openpgpjs
  - connorelsea 8y ago
    Implying in any way that all JS programs are insecure and all other languages are secure is ridiculous and probably harmful, leading people to insecure choices in other languages. I've seen plenty of security vulnerabilities in strongly typed languages in my days as a software dev
  - colejohnson66 8y ago
    OpenSSL? Remember Heartbleed?
    - 8y ago
saagarjha 8y ago
JavaScript may have many problems, but I don't really think security is one of them. In a properly isolated sandbox, such as a web browser, it's much more difficult to gain arbitrary code execution privileges than a native desktop app.
- fjsolwmv 8y ago
  Then JavaScript is a huge problem, only made safe when wrapped in a professionally built and battle tested bubble. OpenBSD is a famously secure unix(alike) distro. That doesn't mean that every piece of software in OpenBSD is safe to use in any other context.
  - dcow 8y ago
    So all code is a huge problem? Makes sense to me.
bigiain 8y ago
Maybe, and I know this sounds crazy, secure chat clients shouldn't execute user/potential-attacker supplied code at all???
- jrochkind1 8y ago
  Of course they shouldn't, that is the bug, I think? The authors thought they were displaying user-supplied HTML, not executing user-supplied code. You can say secure chat clients should not display HTML messages, but that's a pretty different thing.
  - fjsolwmv 8y ago
    Yes the only problem is that the text markup language happens to include by default a Turing complete network-enabled live-interpreted programming language because 25 years ago someone wanted to write a funny message in the Netscape status bar.

by tormeh 8y ago

pfg 8y ago

As much as I'm not a fan of JavaScript, the problem is not so much the language but rather the choice of Electron and all that comes with it. Heck, even a web version or Chrome app would've successfully mitigated these attacks. Electron means you're one XSS away from remote code execution, and even worse, it makes it way harder to mitigate XSS through CSP (which Signal did utilize, but script-src 'self' can easily be bypassed in Electron). FWIW, Signal's native mobile apps are written in Java and Objective-C respectively, so there's not really much of a difference compared to Wire (which is a good choice as well). Still, even a hypothetical React Native app written in JavaScript wouldn't be much worse; after all, React Native isn't just a Web View made to look like a native app, but uses actual native components.

mintplant 8y ago
Signal Desktop actually used to be a Chrome app. Then Google announced the deprecation of that feature and they ported it over to Electron.
- ehPReth 8y ago
  Sad day for Chrome OS users. No more Signal updates! :(
ams6110 8y ago
> Electron means you're one XSS away from remote code execution So, electron is the new flash. I'll be avoiding that, then.
- deleted 8y ago
  [deleted]
rndgermandude 8y ago
While I agree that Electon offers a massive amount of footguns, neither Javascript nor Electon was the issue in this case. The issue was using innerHTML (or rather $.html()) with strings concatenated together from user input. Something you should never do. Could as well just call eval() directly on it, or pass the input to gcc, compile it and run the resulting binary. The Signal devs thought $.html() does some kind of escaping: https://github.com/signalapp/Signal-Desktop/commit/9d41b8616296f1b328aa864e0114b99d7f11ca06 https://github.com/signalapp/Signal-Desktop/commit/9d41b8616... (this commit made something that was easy to exploit into something that was even easier to exploit). To be honest, I'd lay more blame on the authors of the DOM spec making innerHTML a setter than on Electron, and jQuery exposing this misfeature even more with $.html(), teaching an army of web developers to do the wrong thing. We've all seen numerous (XSS) vulnerabilities in all kinds of websites, browser extensions, Electron apps, etc resulting from this API, tho in Electron apps it gets particularly devastating as often you'd get code execution not just in a sandboxed website but full code execution under the current user credentials in the system.
- pfg 8y ago
  So to be clear, a lot of the blame definitely belongs in the "all that comes with it" bucket here, which is one of the reasons why you should think twice about developing desktop apps using a platform that forces you to deal with not only the usual desktop app security concerns, but also all the things that make web apps vulnerable. Still, when you ship an app with a relatively strict Content Security Policy as Signal did (including using script-src 'self'), you don't really expect a simple XSS vulnerability to lead to RCE, but it turns out that policy doesn't really do much in an Electron app.
- caf 8y ago
  Surely the collective noun for footguns is a cache :)
- seba_dos1 8y ago
  >The Signal devs thought $.html() does some kind of escaping: Uhm... that's a really rookie mistake to make. Like, one of the very basics of jQuery usage. I'm not exactly sure what to think about it after seeing this commit you linked...
AgentME 8y ago
>Electron means you're one XSS away from remote code execution, and even worse, it makes it way harder to mitigate XSS through CSP (which Signal did utilize, but script-src 'self' can easily be bypassed in Electron). Can you explain that last part? I can't think of how an XSS attack could get around that, and Electron's documentation specifically recommends it: https://github.com/electron/electron/blob/master/docs/tutorial/security.md#6-define-a-content-security-policy https://github.com/electron/electron/blob/master/docs/tutori...
- pfg 8y ago
  There's a bit of an explanation of this in the article describing the other XSS that's recently been found in Signal[1]. Basically, since the Electron app itself runs under the file:// origin, 'self' can be bypassed with varying degrees of difficulty depending on the platform. On Windows, it's trivial because you can use UNC paths to a SMB share containing a malicious JavaScript file (i.e. file://1.2.3.4/payload.js). On other platforms, you'd need to find a way to place the file on a path accessible via file:// first, for example by sending the file via Signal itself and hoping the user accepts the download. There are ways to lock down the CSP further to mitigate this, but no one really expects script-src 'self' to be unsafe, especially when it's what their documentation recommends. [1]: https://ivan.barreraoro.com.ar/signal-desktop-html-tag-injection/advisory/ https://ivan.barreraoro.com.ar/signal-desktop-html-tag-injec...
burtonator 8y ago
One thing I'd like to see is more use of containers and permissions locally. For example, my IntelliJ runs as my user account, but it doesn't need access to all my files. I should be able to select which directories it has access to and it's within a container by default. I mean I can set this stuff up manually, but in the future I'd like to see this as the default. Similar to the way Android apps ask for permissions.
- kilburn 8y ago
  It seems you are calling for Qubes OS [1], which does that but using VMs (which should be more secure than containers). It will take a looong time for "standard" OSes to get there, if they ever do. The required changes in UX are very significant... [1] https://www.qubes-os.org/ https://www.qubes-os.org/
- pjmlp 8y ago
  That is the whole idea of the UWP model on Windows, and the ongoing work to put Win32 apps inside of the same containers. Or the sandbox models on Android, iOS and macOS.
  - danieldk 8y ago
    And Flatpak on Linux. IIRC the Signal Flatpak is sandboxed.
Rjevski 8y ago
I disagree; the issue is Electron. Sure, you can write secure apps in Electron, just like you can do risky stuff in real-life and be fine most of the time. But why take the risk? Had the app been written with a native language and SDK, they wouldn’t need to worry about escaping or anything. I have yet to hear about getting remote code execution for dumping text into an UILabel or similar, while XSS happens almost every day.

symlinkk 8y ago

None of this is caused because of them using a dynamic language. It's caused because the developers used a function literally called "dangerouslySetInnerHTML" that doesn't escape HTML. That's it. It's just lazy programming.

vjeux 8y ago
We called it that way in React in order to call out attention to the fact that it was actually dangerous. React also properly escapes everything else it prints. The app isn't using React but jQuery, which doesn't have those protections.
- deleted 8y ago
  [deleted]
- archgoon 8y ago
  This doesn't seem to be true, here's the v1.10.0 code: https://github.com/signalapp/Signal-Desktop/blob/f6eb745632ce87eabce93d3c12f66f0bc6146d2d/ts/components/conversation/Quote.tsx#L114 https://github.com/signalapp/Signal-Desktop/blob/f6eb745632c... They do seem to be using react, and using dangerouslySetInnerHTML. Now that said, I haven't confirmed that this is the code that caused the issue, but it is in the Quotes component, which is referenced in the article. They seem to have fixed this specific issue a few days ago (v.11.0): https://github.com/signalapp/Signal-Desktop/blob/0d00fbfb7a23a9b05ea9e0c74910d31a27e710de/ts/components/conversation/Quote.tsx https://github.com/signalapp/Signal-Desktop/blob/0d00fbfb7a2... They do seem to be using jQuery elsewhere in the code base, but I'm not familiar enough to determine how it all fits together.
  - ddtaylor 8y ago
    They are using many frameworks mixed together like jQuery, React, and underscore.
  - vjeux 8y ago
    Oh sorry you are right! I was reading the comment ( https://news.ycombinator.com/item?id=17097501 https://news.ycombinator.com/item?id=17097501 ) which said that the issue was caused with $.html

connorelsea 8y ago

there is plenty of secure software written in javascript. poor engineering can occur in any language

4ad 8y ago
Can you provide some examples?
- kenforthewin 8y ago
  openpgp.js, professionally audited several times over https://openpgpjs.org/ https://openpgpjs.org/
  - na85 8y ago
    Audited by whom? Where are their findings?
    - connorelsea 8y ago
      Googled it for you. From their github repo... "To date the OpenPGP.js code base has undergone two complete security audits from Cure53. The first audit's report has been published here." https://github.com/openpgpjs/openpgpjs https://github.com/openpgpjs/openpgpjs
- connorelsea 8y ago
  Implying in any way that all JS programs are insecure and all other languages are secure is ridiculous and probably harmful, leading people to insecure choices in other languages. I've seen plenty of security vulnerabilities in strongly typed languages in my days as a software dev
- colejohnson66 8y ago
  OpenSSL? Remember Heartbleed?
  - 8y ago

saagarjha 8y ago

JavaScript may have many problems, but I don't really think security is one of them. In a properly isolated sandbox, such as a web browser, it's much more difficult to gain arbitrary code execution privileges than a native desktop app.

fjsolwmv 8y ago
Then JavaScript is a huge problem, only made safe when wrapped in a professionally built and battle tested bubble. OpenBSD is a famously secure unix(alike) distro. That doesn't mean that every piece of software in OpenBSD is safe to use in any other context.
- dcow 8y ago
  So all code is a huge problem? Makes sense to me.

bigiain 8y ago

Maybe, and I know this sounds crazy, secure chat clients shouldn't execute user/potential-attacker supplied code at all???

jrochkind1 8y ago
Of course they shouldn't, that is the bug, I think? The authors thought they were displaying user-supplied HTML, not executing user-supplied code. You can say secure chat clients should not display HTML messages, but that's a pretty different thing.
- fjsolwmv 8y ago
  Yes the only problem is that the text markup language happens to include by default a Turing complete network-enabled live-interpreted programming language because 25 years ago someone wanted to write a funny message in the Netscape status bar.