Last Comment Bug 651081 - Ensure GCLI commands are executed securely
: Ensure GCLI commands are executed securely
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Joe Walker [:jwalker] (needinfo me or ping on irc)
:
Mentors:
Depends on:
Blocks: GCLI-ENABLE
  Show dependency treegraph
 
Reported: 2011-04-19 04:34 PDT by Joe Walker [:jwalker] (needinfo me or ping on irc)
Modified: 2011-06-21 02:46 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-04-19 04:34:29 PDT
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 2011-04-19 08:14:40 PDT
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 2011-04-19 08:19:05 PDT
(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] (needinfo me or ping on irc) 2011-04-19 08:23:15 PDT
(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] (needinfo me or ping on irc) 2011-05-25 06:20:07 PDT
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] (needinfo me or ping on irc) 2011-05-31 00:48:23 PDT
Jesse - Are you best person to talk to about getting this command line feature reviewed?
Thanks,
Comment 6 Jesse Ruderman 2011-05-31 08:47:10 PDT
curtisk has been organizing security reviews lately.
Comment 7 Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]] 2011-05-31 11:59:52 PDT
Review TBD added to sec team review radar https://wiki.mozilla.org/Security/Radar/Active#Firefox:_In_Progress
Comment 8 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-06-16 04:31:28 PDT
Bug 664693 tracks the documenting of the commands.
Comment 9 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-06-16 04:45:42 PDT
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]] 2011-06-16 08:53:19 PDT
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] (needinfo me or ping on irc) 2011-06-16 09:35:44 PDT
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] (needinfo me or ping on irc) 2011-06-21 02:46:08 PDT
Marking verified because there is no UI proof that the bug is fixed. The proof is in the comments above.

Note You need to log in before you can comment on or make changes to this bug.