Disabled context isolation is a security hole/please enable

[Thaodan]

Describe the bug For #434 and further #492 context isolation was disabled. This is a security issue as it exposes all electron api to websites running inside Electron.

If I understand correctly one has to implement contextBridges to be able to use ipcRender which is required for WebRTC. I've linked a few documentation pages around this below.

Desktop :

• OS: any
• Installation package: any
• Version: >1.0.0.17

Additional context

• Related issue: #492
• Commit in question: https://github.com/IsmaelMartinez/teams-for-linux/commit/6e8782b41016e628b69c73de8a4422d175105701
• Electron on Context Isolation
• Electron on contextBridge
• WebRTC-hid-SDK on Context Isolation

[IsmaelMartinez]

I am sorry but currently I don't have time to look at this in more detail. It does look quite complex to re-place unfortunately.

[IsmaelMartinez]

I had a look and it does look a few changes are needed but it should be hopefully double. It might take me a few tries and might break some functionality but I will try 😄 . Thanks for reporting

[IsmaelMartinez]

Ok, this is going to be a much bigger task. It will require to re-work all the parts in the UI. Ideally we can get rid of that one, nodeIntegration and the sandbox, but is a mamouth task.

Electron has deprecated the BrowserWindow so I will try to start the work to move to the BaseWindow and WebContentView, but it might take some time.

In the meantime, I will make this values as config options so people can disable them. They will break functionality for sure, but at least you can still use the app. Not sure when I will have the time to make those changes (the enable them as flags) but hopefully soon!

Thanks for reporting again.

[IsmaelMartinez]

by the way, we do disable eval so that should, in theory, block most of the bad actors. But agree better to make sure we only expose what we have to.

[IsmaelMartinez]

1.8.0 have the ability to disable the contextIsolation and sandboxing in the main BrowserWindow. Currently not doing this until is thoroughly tested by multiple users.

@Thaodan , are you able to test this and see what functionality you see missing? Difficult for me to test from a MacOS it looks like most things are working (all that I tried, but I haven't tried things like custom backgrounds)

Thanks!

[IsmaelMartinez]

another option included is running the app with firejail https://github.com/IsmaelMartinez/teams-for-linux/blob/develop/README.md#running-teams-for-linux-in-a-firejail

contextIsolation and sandbox seem to be breaking screensharing. To fix that we would need to re-architect the app (something that would happen over time, but would require a lot of work).

[nfp0]

How severe is this hole? Could you please give an example of what a bad actor could do in this case?

[IsmaelMartinez]

In theory you could use some of the electron APIs, but I did try to see if I could exploit it and couldn't get much of it, but I am no security expert.

You can disable context isolation etc, there is a config option. That would break screen sharing, but the rest should still work.

I wasn't able to see the "exposed" APIs anywhere, what I assume is that you need to still expose them, something we don't do.

If you are concerned, use firejail, or snap/flathub, that do run on their own context.

I have been trying to re-write all that area, but the logo trademark, and other user issues/requests has kept me busy. Happy to help anyone interested in fixing this. Otherwise it will need to wait until I get time.

[Thaodan]

From what I understand it is not directly about an attacker to the app directly but any attacker or bug that attacks the teams web app.

However since the chromium sandbox is still active it is not as bad.

[IsmaelMartinez]

Aye, we do get monitored by snyc, and github, so if there is a security issue in one of the libraries I will get alarms. (anyone is welcome to subscribe to them, you just need to go to the snyc link in the project)