search

lloeki / matterfront

Mattermost frontend app for OS X, Windows and Linux
MIT License
152 stars 27 forks source link

Add chrome config flags #54

Closed geekytime closed 8 years ago

geekytime commented 8 years ago

Adds basic config options for the Electron chrome args.

geekytime commented 8 years ago

Closing this for now. I'm going to work in my fork for a bit while I get some of these new features solidified.

geekytime commented 8 years ago

What about setting a flags property on the chrome-args object, that takes an array of flags?

These are literally key-value pairs. Why would we want to use an array for that? In the case of these flags, the key is "no-proxy-server" and the value is empty/"". Why is that a problem?

LongLiveCHIEF commented 8 years ago

For the same reason using a boolean didn't sound right to you, (because it will cause confusion), an empty string doesn't sound right to me. Having a "" feels like a placeholder for something, when anything you put there will be ignored.

geekytime commented 8 years ago

Having a "" feels like a placeholder for something, when anything you put there will be ignored.

It's not ignored, it's passed as the value for the flag.

LongLiveCHIEF commented 8 years ago

The value isn't utilized, so it wouldn't matter if it was a boolean, or a string. To me, that just smells of a potential security hole. It's a place where someone could sneak in a function and distribute a build that had nefarious purposes.

By making it an array, we use a more appropriate construct for the type of data, and reduce the likelihood of introducing security flaws.

:-1:

geekytime commented 8 years ago

Feel free to submit a PR demonstrating the vulnerability.

LongLiveCHIEF commented 8 years ago

you-know-it-to-be-true

geekytime commented 8 years ago

Oh, well, if Darth Vader agrees with you, then it must be right. :smile_cat:

LongLiveCHIEF commented 8 years ago

Challenge accepted....

I'm going to use this vulnerability to make the app load up Fox News (as a placeholder for [displaying/loading dangerous site/resource]. Would that be an acceptable demonstration of vulnerability?

geekytime commented 8 years ago

How is that a vulnerability? If people want to ruin the app, there are lots of ways to do that...

geekytime commented 8 years ago

We need to be able to provide quite a bit of flexibility of configuration for the Chromium instance, due to various network configurations. If you can make an argument that a particular flag is too dangerous to allow through, I'm willing to blacklist certain flags, but that's a different discussion than whether the config should be an array.