651081 - Ensure GCLI commands are executed securely

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Joe Walker [:jwalker]]

GCLI commands are plain JavaScript. In GCLI they are executed simply using command.exec();. It is likely that this represents a security problem in some cases since commands will be executed with chrome privileges.

We should ensure that canon.exec() or canon.addCommand() performs the required wrapping to ensure that the correct privileges are used at all times.

This is not considered a "security issue" right now because:
- "Many users" are not using this code
- The only commands that exist don't do much of any note

This issue should be fixed before any significant number of users begin testing it.

Comment 1

[Shawn Wilsher :sdwilsh]

Did this feature go through security review when it first landed?  If not, it'd probably be a good idea to have it do that regardless now.

Comment 2

[David Dahl :ddahl]

(In reply to comment #0)
> GCLI commands are plain JavaScript. In GCLI they are executed simply using
> command.exec();. It is likely that this represents a security problem in some

I have not yet looked at the patch, but, You will have to execute these commands in a sandbox, just like the existing command line.

Comment 3

[Joe Walker [:jwalker]]

(In reply to comment #1)
> Did this feature go through security review when it first landed?  If not, it'd
> probably be a good idea to have it do that regardless now.

It's not landed yet.

Comment 4

[Joe Walker [:jwalker]]

My current thinking is that it would be good if we can have the command line execute with chrome privs, protecting it from page resources, rather than the other way around (i.e. executing with page privs, Sandboxed from chome resources).
Not sure if that's possible.

Comment 5

[Joe Walker [:jwalker]]

Jesse - Are you best person to talk to about getting this command line feature reviewed?
Thanks,

Comment 6

[Jesse Ruderman]

curtisk has been organizing security reviews lately.

Comment 7

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Review TBD added to sec team review radar https://wiki.mozilla.org/Security/Radar/Active#Firefox:_In_Progress

Comment 8

[Joe Walker [:jwalker]]

Bug 664693 tracks the documenting of the commands.

Comment 9

[Joe Walker [:jwalker]]

Bug 664696 tracks the reviewing of the commands by mrbkap

The notes from the etherpad have gone - will they be published anywhere?

I'd like to close this bug now - any objections?

Comment 10

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

mrbkap is supposed to review the wrapper implementation

security team will review the list of commands

Notes are posted here: https://wiki.mozilla.org/Security/Reviews/Firefox6/ReviewNotes/GCLI

Comment 11

[Joe Walker [:jwalker]]

I've added the bugs I raised to the wiki page - thanks for posting that.
I'll close this bug tomorrow unless anyone complains.
Many thanks.

Comment 12

[Joe Walker [:jwalker]]

Marking verified because there is no UI proof that the bug is fixed. The proof is in the comments above.