400227 - [10.5] Extensions containing native scripts or executables will trigger a security warning

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Tony Chung [:tchung]]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a9pre) Gecko/2007101704 Minefield/3.0a9pre
Build Identifier: 

on mac leopard, attempting to install flashgot plugin off of AMO, and restarting the browser, will hang for awhile, and then prompt warning notifications for flashgot.  This doesnt reproduce with other plugins found on AMO so far.   Since this is not a common user experience, i am not certain if this is a security feature for Leopard, or a bug.

See screenshots

Reproducible: Always

Steps to Reproduce:
1. install 2.0.0.8 on leopard
2. go to AMO, and install Flashgot extension
3. Restart firefox
4. Verify it takes awhile for it to load up again, and two flashgot warnings popup first, instructing the user to open or cancel.

** Doesnt repro on Mac Tiger OS
Actual Results:  
2 warning dialogs appear and action needs to be taken before proceeding

Expected Results:  
No warnings

Comment 1

[Tony Chung [:tchung]]

Attachment: FlashGot Unix warning screenshot

Comment 2

[Tony Chung [:tchung]]

Attachment: FlashGot macSh warning screenshot

Comment 3

[Daniel Veditz [:dveditz]]

Does Leopard add something like Vista's UAC? Maybe this is expected for the few add-ons with binary executables.

Comment 4

[Marcia Knous [:marcia]]

Yes they do, but unfortunately in this case if you click cancel you get in a really horked state. You get the dialog the first time you run an app after it has been downloaded.

(In reply to comment #3)
> Does Leopard add something like Vista's UAC? Maybe this is expected for the few
> add-ons with binary executables.
> 

Comment 5

[Marcia Knous [:marcia]]

As a point of information, you don't get in the horked state if you click "OK" to the two dialogs that you get after launching Firefox.

Comment 6

[Marcia Knous [:marcia]]

Another thing I noticed while running on a PPC mac, you get the two warnings Tony posted **every** time you start Firefox. And since there is a bit of a delay between the warnings coming up, this makes for a very bad user experience.

Comment 7

[Giorgio Maone [:ma1]]

Notice that those scripts are created on the fly inside the temporary folder by FlashGot's JavaScript component.
Is any file written by Firefox on the disk flagged as "downloaded from the Internet"?
Is there any way for an extension to create a shell script and then launch it without triggering this protection?

Comment 8

[Steven Michaud [:smichaud] (Retired)]

> Notice that those scripts are created on the fly inside the
> temporary folder by FlashGot's JavaScript component.

Giorgio, how does FlashGot invoke these scripts?  It's possible that
the problem may go away if you change the way they're invoked.

My reasons for saying this are as follows:

If you download a Firefox DMG and drag a copy of Firefox off the DMG
to somewhere on your hard drive, you'll get Leopard's warning if you
run it by double-clicking on it.

But this doesn't happen if you run "firefox-bin" from the Terminal
command line.

And something very interesting happens if you try "open Firefox.app"
from the Terminal command line:  The app seems to hang as it starts
up, as if the warning were being displayed (though in fact it isn't
being displayed).

Are you by any chance using "open" to invoke your scripts?

Comment 9

[Giorgio Maone [:ma1]]

(In reply to comment #8)
> Are you by any chance using "open" to invoke your scripts?

1. I create the file by with nsILocalFile.create(0, 0700).
2. I fill it using nsIFileOutputStream.
3. I use it to init a nsIProcess instance, then call run() on it: this delegates to LaunchApplication(), as seen in http://mxr.mozilla.org/seamonkey/source/xpcom/threads/nsProcessCommon.cpp#308

Again, is there any way to "untag" the file, or to create it in a special way  that flags it as "NOT downloaded from the internet"?

Comment 10

[Steven Michaud [:smichaud] (Retired)]

> this delegates to LaunchApplication()

I'd bet it's LaunchApplication() that's causing the trouble (that's
behaving like "open", or like double-clicking in the UI).

> Again, is there any way to "untag" the file, or to create it in a
> special way that flags it as "NOT downloaded from the internet"?

As far as I know there isn't.  And I doubt that "tags" of any kind are
involved -- I suspect the OS has a database of programs that are
allowed to run.

Which (if I'm right) raises the possibility of adding your plugin (at
least) this database.  But it wouldn't be feasible to add "scripts"
that are created on the fly.  And in any case I know of no documented
way to do this.

Comment 11

[Giorgio Maone [:ma1]]

(In reply to comment #10)

> Which (if I'm right) raises the possibility of adding your plugin (at
> least) this database.  But it wouldn't be feasible to add "scripts"
> that are created on the fly. 

This would be quite unfortunate, as at this moment nsIProcess::Run() on the Mac cannot pass any argument to the invoked process, and the only feasible work-around I'm aware of is creating a custom launcher script for each arguments combination and invoking it.

Comment 12

[Steven Michaud [:smichaud] (Retired)]

Apple hasn't (yet) made much information available on the so-called
"tagging" of so-called "downloaded applications".  I'll discuss what
I've been able to find in more detail in my next message.

But I'm already pretty sure there's nothing that can (reasonably) be
done on the browser side about this bug (400227).

Giorgio, I don't believe you'll be able to use nsIProcess::run()
(because it uses LaunchApplication()), or even (probably)
nsILocalFile::launch() (because it uses LSOpenFSRef()).

I'm quite sure (from your results with nsIProcess::run()) that
LaunchApplication() has a "hook" for Leopard's new code that puts up
the "are you sure you want to open it" dialog.  I suspect Apple has
done the same with LSOpenFSRef() ... though it'd be worth trying
nsILocalFile::launch() to be sure.

Probably your only option is to use some variety of the C exec()
command.  You'd need to be able to get a full path from the
nsILocalFile interface ... but I assume this is possible.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

> Apple hasn't (yet) made much information available on the so-called
> "tagging" of so-called "downloaded applications".

As best I can tell, all that Apple has to say on this subject can be
found here:

http://www.apple.com/macosx/features/300.html#security

Though the blurb does talk about "tags", I don't believe (as I said
earlier) that any alterations are made to "downloaded" files.
Instead, the information is almost certainly stored in one or more
databases somewhere in the file system.

OS X has long had a "Launch Services" database -- about which more is
known because it's been out longer and people have had a chance to dig
into it.  My first guess was that this'd be where Apple stores the new
information ... but that's only partially correct.

You can dump the entire contents of the Launch Services database (in
readable format, to stdout) by running "lsregister -dump".  (The
lsregister command is located in the /System/Library/Frameworks/
CoreServices.framework/Versions/A/Frameworks/LaunchServices.framework/
Versions/A/Support/ directory on Leopard and in the /System/Library/
Frameworks/ApplicationServices.framework/Versions/A/Frameworks/
LaunchServices.framework/Versions/A/Support on Tiger.)

Only app bundles are listed here, but you'll notice that ones which
haven't ever been run in Leopard have a "launch-disabled" flag.  This
doesn't (by itself) determine whether the warning will be displayed
when the app is first run (some other database is also consulted).
But no app bundle that _has_ been allowed to run still has this flag
set.

As I said above, there appear to be ways to bypass Apple's
downloaded-file-warning hook.  For example you'll never see the
warning if you run "firefox" from the Terminal command line (this is
my main reason for thinking that you might also be able to avoid the
warning by using C exec()).

And (as we've found with this bug) a file needn't have been
"downloaded" for Leopard to display a warning before it's run for the
first time.  It appears that any file _created_ by certain kinds of
apps is considered to have been downloaded.  Both Firefox and Safari
have a "quarantined" flag set in the Launch Services database.  I
suspect (but don't yet know) that this is why the OS considers files
created by Firefox (and presumably also Safari) to have been
"downloaded".  In principle you might change Firefox's Launch Services
database entry to get rid of the "quarantined" flag.  But (aside from
the fact that I'm not sure this is possible for third parties) I think
this would be unsafe, and a very bad idea.

Another oddball piece of information (not relevant, but possibly
interesting):  If you have Tiger installed on another partition on the
same computer where you've installed Leopard, files downloaded in
Tiger don't show the "downloaded file" warning when you first run them
in Leopard.  At first I thought that FlashGot's behavior might change
if its *.xpi hadn't been downloaded in Leopard, but that turns out not
to be true.  FlashGot's problem is that it creates files and then runs
them, using a "hooked" API.  And because FlashGot is running "inside"
Firefox, the OS thinks that Firefox "downloaded" these files.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

This bug should ultimately be marked INVALID, since it's not really a
problem with Firefox.  But I'll leave it open for the time being.

Comment 15

[Stuart Morgan]

If you have downloads to specific places that you believe are completely trusted, you can opt those locations out of the quarantine system (see bug 393515 for an example), but you'd want to be very careful about doing that.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Thanks, Stuart.  So my guess about bug 393515 was correct.

But I can't get the Info.plist syntax (from that bug) working in
Firefox.  I suspect that Apple has simply not (yet) implemented the
LSFileQuarantine "exclusions" ... at least not as of build 9A559 (one
release before the gold master, which is build 9A581).  (I'll dig into
this further next week.)

Stuart, have you been able to devise a test to see if Camino's
"exclusions" are working?

Comment 17

[Giorgio Maone [:ma1]]

Could someone having Leopard at hand (I don't) verify if the work-around I just implemented in http://flashgot.net/betas/flashgot-0.6.9.4.xpi actually works?
It should make Leopard warn about the "FlashGot" script application just once, after the first Firefox restart, and then should operate quietly for ever.

Many thanks!

On a side note, can we hope bug 68702 brings good news about the general issue?

Comment 18

[Steven Michaud [:smichaud] (Retired)]

> It should make Leopard warn about the "FlashGot" script application
> just once, after the first Firefox restart, and then should operate
> quietly for ever.

I just tried FlashGot 0.6.9.4 in Leopard client build 9A559, and it
makes no difference -- I get exactly as many warnings as previously.

Which is two warnings when you restart Firefox after having installed
FlashGot, and afterwards one warning each time you start Firefox.  To
make this especially annoying, there's about a 30-second delay before
each warning (same as previously).

I have some hope of getting what was discussed in comment #15 and
comment #16 to work ... but you probably shouldn't count on this.

As I said previously, you should at least try adding some C/C++ code
to FlashGot to call exec() (or one of its friends) on the contents of
the nsILocalFile objects you create.

> On a side note, can we hope bug 68702 brings good news about the
> general issue?

You mean some time this century? :-)

(Someone else can chime in if I'm being unfair.)

Comment 19

[Giorgio Maone [:ma1]]

Steven, could you tell me what exactly those warning say and how you dismiss them (Cancel VS Open)?
If you approve the "FlashGot" script, it's very strange you get the warning again because my rework is about making it immutable, i.e. it doesn't get rewritten on each startup as it was before...

Comment 20

[Steven Michaud [:smichaud] (Retired)]

> Steven, could you tell me what exactly those warning say and how you
> dismiss them (Cancel VS Open)?

They're the same warnings as previously (with one minor difference
that I'll describe below).  I always click on Open.

> it doesn't get rewritten on each startup as it was before...

This quite apparently isn't true -- the warnings always give a new
timestamp when they say "Firefox downloaded this file today at ...".

(The second warning (the one you get on restart after installing
FlashGot, and subsequently whenever starting Firefox) is now worded
slightly differently: xInstead of saying '"flashgot-mac.sh is a script
application which was downloaded from the internet', it now says
'"FlashGot" is an application which was downloaded from the internet".)

Comment 21

[Giorgio Maone [:ma1]]

Steven, thank you very much for testing.
Could you try http://flashgot.net/betas/flashgot-0.6.9.5.xpi as well?

Comment 22

[Steven Michaud [:smichaud] (Retired)]

> Could you try http://flashgot.net/betas/flashgot-0.6.9.5.xpi

OK.  FlashGot 0.6.9.5 did what you hoped:

I only got one warning (the '"FlashGot" is an application ...'
warning) on restarting Firefox just after the installation, and no
further warnings.

There's still a ~30-second delay before that one warning -- which
makes this solution still not ideal.  But things are certainly better
than before.

By the way, I suspect that 30-second delay is a deliberate attempt by
Apple to frustrate hackers.  I've never seen it with "ordinary"
downloads.  And I bet you wouldn't see it if the FlashGot script's
timestamp were sufficiently older than the current time.

Do you have any way to manipulate the FlashGot script's timestamp to
make it seem older?  This wouldn't get rid of the warning, but it
might (I hope) get rid of the 30-second delay.

Comment 23

[Steven Michaud [:smichaud] (Retired)]

> And I bet you wouldn't see it if the FlashGot script's timestamp
> were sufficiently older than the current time.

I just realized that if Apple were smart about this, they'd get the
timestamp from their "downloads" database -- not from the file itself.

But it's still worth a try.

Comment 24

[Steven Michaud [:smichaud] (Retired)]

> But I can't get the Info.plist syntax (from bug 393515) working in
> Firefox.  I suspect that Apple has simply not (yet) implemented the
> LSFileQuarantine "exclusions" ... at least not as of build 9A559
> (one release before the gold master, which is build 9A581).

Actually, I've now found that the syntax _does_ work, but only in a
very half-assed way:  You may only specify one exclusion.  If you
specify more than one, neither will work.

So the following syntax (from bug 393515) doesn't work at all:

  <key>LSFileQuarantineExcludedPathPatterns</key>
  <array>
    <string>~/Library/Caches/*</string>
    <string>~/Library/Application Support/*</string>
  </array>

But either of the following works just fine:

  <key>LSFileQuarantineExcludedPathPatterns</key>
  <array>
    <string>~/Library/Application Support/*</string>
  </array>

  or

  <key>LSFileQuarantineExcludedPathPatterns</key>
  <array>
    <string>~/Library/Caches/*</string>
  </array>

If we are only able to specify one exclusion, it will need to be as
follows (which also works):

  <key>LSFileQuarantineExcludedPathPatterns</key>
  <array>
    <string>~/Library/*</string>
  </array>

This is almost certainly an Apple bug.  I hope it's fixed in the gold
master (build 9A581).  But if it isn't, I think we can live with this
setting for LSFileQuarantineExcludedPathPattern.

Comment 25

[Steven Michaud [:smichaud] (Retired)]

> http://www.apple.com/macosx/features/300.html#security
>
> Though the blurb does talk about "tags", I don't believe (as I said
> earlier) that any alterations are made to "downloaded" files.
> Instead, the information is almost certainly stored in one or more
> databases somewhere in the file system.

Though I was right that the "tags" aren't stored in the "downloaded"
files themselves, I was wrong about this information being stored in a
"database" (aside from the Launch Services database).

Instead the information is stored in the file system itself, as an
extended attribute of the "downloaded" file.

To see this in action:

1) Install FlashGot (0.6.9.3) into Firefox (any version) and wait
   until the OS displays a warning about a "downloaded" file.  Don't
   (for the time being) dismiss the warning).

2) Open a Terminal prompt and enter the following command (to find
   where the "downloaded" file is located).  Use the same name as the
   file you're being warned about:

   find . -name flashgot.fgt

    or

   find . -name flashgot-mac.sh

3) Once you find where the file is located, run the following command
   on it in your Terminal prompt:

   xattr -l path/to/file

   Here's an example from my own home directory:

   xattr -l
     "Library/Caches/TemporaryItems/flashgot.wjpmgk1c.default/flashgot.fgt"

   On my system this command returns:

   com.apple.quarantine: 0000;4727bd8b;Firefox;|org.mozilla.firefox

   "com.apple.quarantine" is the "name" of the extended attribute.
   The rest of the line is the extended attribute's "value".  I don't
   know what the first part of the "value" is (0000), but I suspect
   the second part (4727bd8b) is a timestamp.

4) Dismiss the warning and run the command from step 3 again.  Now you
   should find that the file no longer has any extended attributes.

Leopard's extended attributes are discussed in more detail at:

http://arstechnica.com/reviews/os/mac-os-x-10-5.ars/11

Comment 26

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

The problem I described in comment #24 still happens on the Leopard
release (build 9A581).  So here's a patch that implements the solution
I proposed there.  This is a 1.8-branch patch, but it should (I think)
also apply to the trunk (with some offsets).

And here's a BonEcho tryserver build that implements the patch.
Giorgio (and whoever else has seen this problem), please try
installing FlashGot 0.9.6.3 in this build.  I think you'll find that
all the warnings have disappeared.  Please let us know your results.

https://build.mozilla.org/tryserver-builds/2007-11-05_09:[e-mail]-bugzilla400227/[e-mail]

Comment 28

[Marcia Knous [:marcia]]

Steven: Unfortunately the build cited in Comment 26 gives me a 404 error :(

(In reply to comment #26)

Comment 29

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #28)

That's because tryserver builds get deleted after 30 days.

I'll build another sometime in the next few days.

Comment 30

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #29)

Here's a new build, made with the same patch:

https://build.mozilla.org/tryserver-builds/2007-12-13_08:[e-mail]-bugzilla400227/[e-mail]

Comment 31

[Marcia Knous [:marcia]]

Confirming that the build cited in Comment 3 (it is a branch build) does address the issue of the security warning coming up. I was running on 10.5 when I tested.

Comment 32

[Marcia Knous [:marcia]]

During testing of RC on Leopard I see this bug, so I am commenting here to get this back on the radar.

Comment 33

[Shawn Wilsher :sdwilsh]

Hate to add more issues to this bug, but this has come up in at least one other place so far as well.  I had someone contact me about an issue they had.  The image looks to be the same as the one attached to this bug:
http://skitch.com/jerome/p69a/firefox-growl-annoyance
The file in question is in /var/folders/iR/iR7JYHkOFHaM9zOU0db5VU+++TI/-Tmp-/ on this system.

Anyway, to ensure this doesn't happen here as well?

Comment 34

[Steven Michaud [:smichaud] (Retired)]

As I mentioned in comment #26, this problem has an easy fix.

But the fix has potential security implications, and so needs to be
reviewed by someone who's willing/able to address them (in addition to
Josh).

What my patch actually does is to specify an "exclusion" from the
normal LSFileQuarantine behavior -- a set of directories (matching a
filespec-with-wildcard) where the browser can create files (and then
run them) without triggering the "are you sure you want to open it?"
warning.

As I say in comment #26, Leopard has (or had) a bug that prevented the
documented exclusion syntax from always working correctly -- as of
that comment's date, you could specify at most one
filespec-with-wildcard.  I need to check if this is still true.  But
whether or not it's still true, I can't forsee any trouble (or
security issues) arising from the single exception specified in my
patch from comment #26.

In either case, though, we really need to hear from someone who knows
more than I do about how the browser creates files (and sometimes
"executes" them).  Any suggestions who I should ask to do the security
review?

Comment 35

[Steven Michaud [:smichaud] (Retired)]

Side note:  The equivalent of my comment #26 patch was landed in
Camino long ago (see bug 393515).

Comment 36

[Marcia Knous [:marcia]]

Perhaps dveditz for the the security review?

Comment 37

[Shawn Wilsher :sdwilsh]

dveditz or shaver perhaps?  We could manually mark downloaded files as ones that should be quarantined (bug 401744).

Comment 38

[Steven Michaud [:smichaud] (Retired)]

> We could manually mark downloaded files as ones that should be
> quarantined

I'm not sure that's possible:  Judging by what I've seen at bug
401744, bug 401748 and elsewhere, you can stop a particular download
from being quarantined, or add customized information to the
quarantine info.  But I'm not sure you can _make_ a download be
quarantined.

And in any case it'd be much simpler to go at things the other way --
let the OS quarantine most "downloads" as expected, and make
exceptions as needed.

(By the way, it's not "downloads" per se that are quarantined, but all
files created by the browser (unless exceptions are made).)

Comment 39

[Steven Michaud [:smichaud] (Retired)]

Thanks, Marcia and Shawn, for the suggestions about security reviewers.

I think I'll do a new patch (or at least a new tryserver build), and then ask for the review.

Comment 40

[Shawn Wilsher :sdwilsh]

Note, one of the growl developers did more research into the issue I brought up (which may not be this bug) and filed a radar ticket:
http://groups.google.com/group/growldiscuss/msg/84d3d0a213fc22fa?hl=en

Comment 41

[Marcia Knous [:marcia]]

Just wanted to note that I the unfortunate side affect of this bug is that if you don't see the prompt and choose "Open" after installing the extension you end up having to force quit Firefox. Also I suggest updating the Litmus test case to include this bug - I will do that now.

Comment 42

[Josh Aas]

Comment on attachment 287429 [details] [diff] [review]
Fix

Looks OK to me but I'd like to have dveditz's opinion too.

Comment 43

[Daniel Veditz [:dveditz]]

Comment on attachment 287429 [details] [diff] [review]
Fix

I'm sympathetic to the problems one specific addon is having, but I'm not sure I'm comfortable defeating an OS security measure for an entire shared directory tree.

Restricting it just to the Firefox profile wouldn't be much better. What if there's some other addon that helps the user download stuff and later launch it, and stores it in the user's profile? The Leopard warning would be appropriate in that case.

Did Apple ever fix their one-directory limitation? It would be much more appropriate to individually whitelist only the extension directories for those addons which need this. Are apps allowed to change their Info.plist on the fly? If so we could even add built-in support for extensions to register themselves if they need this functionality.

Comment 44

[Steven Michaud [:smichaud] (Retired)]

> Did Apple ever fix their one-directory limitation?

Not that I know of, but I'll check.

> Are apps allowed to change their Info.plist on the fly?

No.

Comment 45

[Stuart Morgan]

(In reply to comment #43)
> Did Apple ever fix their one-directory limitation? It would be much more
> appropriate to individually whitelist only the extension directories for those
> addons which need this. Are apps allowed to change their Info.plist on the fly?
> If so we could even add built-in support for extensions to register themselves
> if they need this functionality.

Quarantine flags can be programatically removed (mento added support for that to Sparkle, for example, so our signed and validated updates wouldn't annoy users with quarantine warning), so while you can't dynamically opt directories out, Firefox--or extensions with compiled code--could remove the quarantine attribute from specific things before opening them if it knows they are safe.

Alternately:
(In reply to comment #38)
> But I'm not sure you can _make_ a download be quarantined.

Our experience is that adding the quarantine metadata to a download is sufficient to quarantine it. We had to conditionalize Camino's code for the equivalent of bug 401748 to prevent quarantining files that the OS had not quarantined.

Comment 46

[Giorgio Maone [:ma1]]

OK, since the old nsIProcess.run() bug preventing arguments from being passed on Mac OS X is fixed in Gecko >= 1.9.0, in current FlashGot dev build (1.1.8.9, http://flashgot.net/getit#devel ) I'm working around this way:

<code>

var xattr =  CC["@mozilla.org/file/local;1"]
             .createInstance(CI.nsILocalFile);
xattr.initWithPath("/usr/bin/xattr");
var args = ["-d", "com.apple.quarantine", "path/to/my/executable"];
var proc = CC['@mozilla.org/process/util;1']
          .createInstance(CI.nsIProcess);
proc.init(xattr);
proc.run(false, args, args.length, {});

</code>

This can be done by any extension which needs to, even if they are pure JS.
Worth to be documented somewhere?

Comment 47

[Steven Michaud [:smichaud] (Retired)]

Not bad!

> Worth to be documented somewhere?

Yes, I think so.

For those who don't know, 'xattr -d attr_name file' deletes a given
"attribute" from a given file.  For another use of xattr, see comment
#25.