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)
:
: J. Ryan Stinnett [:jryans] (use ni?)
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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Jesse Ruderman 2011-05-31 08:47:10 PDT
curtisk has been organizing security reviews lately.
Comment 7 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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.