Desktop app allows PITM to execute arbitrary code

[cathyjf]

There are two separate PITM (person in the middle) issues with the desktop app. The first one is less important for reasons to be explained below.

1. Since the original download of the .exe/Mac app is over HTTP, a PITM can replace the binary by her own crafted version.
2. The index.html file of the app itself merely redirects to http://play.pokemonshowdown.com. A person in the middle can insert arbitrary content into the HTTP stream and execute arbitrary code on the user's computer using the node-webkit functions.

The second problem is actually notably bad because it means that running a safe program (Pokemon Showdown) on an unsafe network can result in the user's computer being totally compromised.

Recommendation: the desktop app should force loading over HTTPS. One issue is that if HTTPS connection fails, the client has always attempted to connect via alternative methods instead (although this was actually broken until my recent fix in @50d07a6020a54c1f6c). We may want to introduce some kind of special flag to indicate not to try HTTP connections, and make sure the desktop app signals that flag.

Regarding the first issue, there's no real solution because even if the binary is served over HTTPS, an attacker can just downgrade the connection to HTTP and serve his crafted binary. We may want to provide an optional cryptographic signature for the application, but that won't really help 99% of users; in order for it to be useful, the user would have to already know that that option exists (since a PITM could strip out the info) and would have to be careful to use it.

Indeed, the first issue is a general problem with downloading binaries over the Internet, except through a package management system. (Package management systems avoid this issue by distributing cryptographic keys as part of the OS.)

Anyway, the second issue is worth addressing by:

1. Making the app use the HTTPS site; and
2. Making the app use some kind of special parameter when loading the HTTPS site so that the client will never attempt to use a non-HTTPS connection (otherwise, an attacker could just block HTTPS traffic).

The chance of the second issue being exploited might seem remote, but we're talking about arbitrary code execution on users' computers so it's worth addressing.

[cathyjf]

Incidentally, a general side effect of the desktop app is that any XSS vulnerability in the client becomes a vector to execute arbitrary code on users' computers -- a single instance of missing sanitisation can result in thousands of users becoming part of a botnet. The desktop app really raises the stakes in general.

[cathyjf]

After making the above comment, I discovered several more XSS vectors, which I fixed in @224a577929b2ae. These vulnerabilities could have been leveraged to completely take over the computers of anybody using the desktop app.

[Zarel]

I've been thinking about this since right after releasing the first version of the client.

You're right that HTTPS would solve a significant number of attack vectors, that was my first thought as well.

The XSS vectors you found weren't exploitable since they would have required owning sim.psim.us first, although for that reason, until I do a security audit of battle.js, I don't plan to have the desktop client support alternate servers.

I'm thinking the client should also have a toName it runs strings passed to Tools.getBlah before setting blah.name, which filters out <, >, and " characters (in addition to the usual toName sanitization).

In the longer term, I'm probably going to replace the current string concatenation battle.js uses with a templating system.

[cathyjf]

The XSS vectors I found were exploitable because there are still various ways to gain control over the sim server -- it's a lot easier than gaining control of the web server. Plus, even if that weren't the case, it would still be a privilege escalation.

[Zarel]

My other thought was to sandbox the website so it could only read/write to ~/Documents/My Games/Pokemon Showdown/ and do some other authorized things, but that's a lot of work. I'll get to it at some point, though.

[Zarel]

Desktop client is HTTPS now.