1163067 - Possible attack vector when forking subprocess during app installation

Status: RESOLVED INVALID

(Firefox Graveyard :: Webapp Runtime, defect)

Description

[(not reading bugmail) Nick Desaulniers [:\n]]

Tim and I were talking about [0], and I mentioned how we were having sanitation issues when we invoke sips as a subprocess for icon generation [1].  Tim and I postulated that it might be possible to have an app named `; rm -rf ~;` that would really mess things up.  I think this would only be a concern on OSX, since we shell out to sips on OSX.  We should make sure it's not a possible attack vector.

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=1152552
[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/webapps/MacNativeApp.js#321

Comment 1

[(not reading bugmail) Nick Desaulniers [:\n]]

A subshell was stripped:

  "name": ";(mou);",

was installed as `mou);` and did not execute anything.

  "name": ";(mou);(mou);",

was stripped and installed as `mou);(mou);`.

  "name": "  ;(mou);(mou);",

was stripped correctly as well.  (mou is a command line utility I've installed that opens a window, thus apparent when run).

   "name": " && mou;", was stripped, as was "name": " || mou;", and "name": "`mou`",.

I'm starting to doubt that this is a possible attack vector and more of a false alarm.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

We're not actually "shelling out" here.  The code in question calls nsIProcess.runAsync, which is implemented by nsProcess::RunAsync <http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/nsProcessCommon.cpp?rev=7bc6ca149561#354>, which eventually (-> nsProcess::CopyArgsAndRunProcess -> nsProcess::RunProcess) executes sips via process_spawnp <http://pubs.opengroup.org/onlinepubs/009695399/functions/posix_spawn.html>.

I suppose sips could itself be shelling out, but that seems unlikely, and in any case it would be a Darwin bug.  Our code looks safe, and Nick's tests confirm that, so I think this is not actually an issue.  I'm marking it as such, but please reopen the bug if you disagree!

(And kudos for flagging it.  I'd much rather we raise anything that worries us, even if the concern turns out to be unwarranted, than ignore it and be blindsided by its discovery in the wild.)