Comment by pilif | Hacker News Reader

Comment by pilif | Hacker News Reader

My main takeaway from this article is the question why conextIsolation was introduced defaulting to false.

This is one huge lever to help with the maxime “XSS is RCE in electron” and yet they default to not helping.

I know this is about backwards compatibility, but they could easily have decided to throw if the property is unset. Security-minded people would have set it to true and dealt with the fallout, whereas others could have set it to false and shipped their update still.

But by defaulting to false, this security tool is hidden from both existing and new users. Old code will not even have the chance to get fixed and new code will be written in an insecure state.

I’m sure the release notes talked about this feature, but who reads release notes? Especially not past release notes (when starting fresh today).

The backwards compatibility cost of throwing and in the message even suggesting setting to false as an emergency out would have been minimal compared to the fallout this is causing.

For what it's worth, 'contextIsolation' will become enabled by default in Electron v12 [0].

[0] https://www.electronjs.org/docs/tutorial/context-isolation#w...

The idea that this is even an option is hilarious. Call it "enableRce" maybe?

Under most modes, I agree the setting should be turned on, but some advanced local only code bases / programs might require this option turned off/on

How common are 'local only' electron apps anymore? I can't think of any program that I've run recently (even a large number of my non-vim terminal applications) which don't make some kind of remote calls.

I can write secure apps even with this option disabled, thank you. Really it's no harder than preventing SQL injection. All I do is that I don't use APIs that can lead to interpreting data as code.

So I assign to textContent liberally, don't manipulate URLs as text, but via browser APIs, and avoid everything that allows code execution. It's really stupid simple to follow safe practices for absolute majority of apps.

And this makes writing a electron app with direct access to local PostgreSQL database trivial, fun, and makes writing useful apps without a separate HTTP backend a breeze.

I don't have a need to split privileged code out to separate process, and go back to acting like there's trusted/untrusted split, just like with regular website. Having to do that would kill all the allure of Electron, for me. I use electron precisely because I don't have to do this, and can depend on the HTML/JS code to be trusted.

> It's really stupid simple to follow safe practices for absolute majority of apps.

That’s what C programmers tell me about memory management and buffer sizing.

And yet: https://www.zdnet.com/google-amp/article/microsoft-70-percen...

Thing is: even if you are careful, you only need to make a single mistake to lose everything.

Just like there’s is benefit in memory-safe languages, there is benefit in environments where a single .innerHTML instead of a .innerText doesn’t mean an RCE.

But that wasn’t really the point of my initial comment. Just as I can accept C programmers thinking they don’t make mistakes, I can accept you believing never making any mistakes either and thus I can even accept electron having a footgun mode.

What I took issue with wasn’t the general availability of such a mode but the fact that this mode is a silently chosen default.

Have people explicitly opt in or out of this safety feature. Don’t assume they want to shoot themselves in the foot by default.

That was my concern and this is the pet I hope we can all agree about.

That I personally also think that turning off context isolation should be impossible is another matter and not really part of this discussion and I’m even willing to accept that people could change my mind about that one.

But having an unsafe default is craziness in my book.

Yes, it doesn't need to be default. But comparing C and JS doesn't make much sense here. It's easy to avoid using innerHTML and similar. You can just turn those functions off, if you don't trust yourself to not type them out from time to time in your editor, or if you don't trust your dependencies to not use them.

There's nothing unsafe about having context isolation off, per se. Yes, you can do stupid stuff like run code directly from the internet, but context isolation doesn't prevent that. It still allows RCE. It just limits the fallout to just stealing your data, like with any other XSS on regular websites.

And if programmers can't be trusted to not execute untrusted input as code in more secure context, adding an extra hop via some IPC will not save them, if they make the same mistakes in less secure context.

Loading a remote page and executing it in Electron is not the only, or even recommended, way to build Desktop apps using Electron. Electron was designed for creating Desktop apps, where the content is shipped in the app itself - in that case, contextIsolation is much less necessary because every way you could break out of the sandbox isn't interesting, it's just an obscure way to hack yourself. It's also a huge compromise to the developer experience, and is a big roadblock to anyone other than Electron experts.

Unfortunately, the world disagrees - they really want to wrap websites using Electron, so now we need to default to options that make Electron apps far less compelling as a platform

I would assume the danger is not your own content breaking out of the sandbox, but displaying unsanitized content from a remote database, which sadly still happens a lot more than you would expect. Definitely not something that only happens when wrapping a website using Electron.

> it's just an obscure way to hack yourself.

that's only true for applications that strictly only ever work on and display content that the user has created themselves.

This is an increasingly rare type of application in the age of the internet and social media to the point where I would argue that most applications these days interact with data created by other users (think opening an image file, displaying a chat message, displaying an email, etc).

With Electron you're setting yourself up in a way where a single case of a forgotten input escaping automatically means a remote code execution attack.

On websites, XSS certainly are a bad thing, but they are still somewhat limited in scope and generic mitigations exists in many browsers.

In Electron apps, an XSS means an RCE unless you make use of features like `conextIsolation`.

Reading your comment makes it obvious to me that this isn't already a well-known issue. People generally aren't aware or believe their type of application to be immune and thus it would, and that's the point of my initial comment, make sense to either default to secure or at least default to asking the user to specifically make the decision to be insecure.

Defaulting to turning every XSS attack into an RCE seems to me like a very bad decision.

The fact that there is an app that downloads privileged code as a normal course of action is already a Remote Code Execution issue to me. Furthermore, the fact that it's only https protected means that any https proxy in between me and the server (say a corporate firewall) can execute random code on my computer. No thanks! And they are just a single hack of their static js hosting away from having all of their active user's computers hacked.

Maybe because they still wanted it to be easy to use? From further down:

> Getting everything to work in a context isolated world was a many month effort, which had begun long before this bug was reported to our team.

-- https://news.ycombinator.com/item?id=24823460

I am well aware of the compatibility issues and the associated difficulties. I wasn't saying the option of turning it off shouldn't be given. I'm saying that the setting should have been mandatory.

People wanting to do it the "easy" way (using scare quotes because turning any XSS into an RCE feels like hard mode to me) set contextIsolation to false and they are done.

Also consider newly created applications: when the limitations exist from the start, it's much easier to build this correctly (the same way it's easier to get to 100% test coverage if you start with TDD than if you have to refactor years of static methods and global state and implicit dependencies).

By having a default setting and having it default to insecure, even newly created applications will be unsafe and will require refactoring in the future.

I hate to be the "RTFM guy," but contextIsolation is pretty well-covered in their docs[1] (and it will be enabled by default beginning with Electron 12).

[1] https://www.electronjs.org/docs/tutorial/security

This might be true, and in certain contexts I’d agree that defaulting to the secure option -while potentially breaking- could be the safer one.

But I have to disagree with your point about reading release notes, this should be the expected behavior from developers working on the product, maybe not every single dependency but at least the main ones. Being an Electron app mainly, discord devs should’ve known and understood what they’re working with. Of course mistakes happen, but the main issue here is developers missed reading the docs properly, not whatever the default value is.

Reply on news.ycombinator.com