804979 - DLL Hijacking - Seamonkey installer

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Frank Wein [:mcsmurf]]

We need to port the patch from that bug to our Windows installer.

+++ This bug was initially created as a clone of Bug #792106 +++

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427

Steps to reproduce:

I downloaded the current Firefox installer in the Windows downloads directory. 
C:\Users\User\Downloads
Then I put a trojan dwmapi.dll in the same directory.


Actual results:

If a user wants to install the Firefox browser, the setup loads the trojan dll with administrative privileges.
As described here http://seclists.org/fulldisclosure/2012/Aug/134 , you can compromise the victim with a social engineering attack like this.



Expected results:

The installer should not load the trojan dll.

Comment 2

[Frank Wein [:mcsmurf]]

Attachment: Patch

I copied the Firefox sfx file and replaced the Firefox strings inside the version info with the help of this http://www.wilsonc.demon.co.uk/d10resourceeditor.htm resource editor.

Comment 3

[Frank Wein [:mcsmurf]]

Comment on attachment 674632 [details] [diff] [review]
Patch

I tested the patch with the instruction from Bug 803515 Comment 2. Without the patch a lot of cmd.exe processes spawn (and can be observed in the task manager). With the patch no cmd.exe processes can be observed in the task manager. I also used Process Monitor to verify the installer no longer loads the proof-of-concept trojan DLL.

Comment 4

[Frank Wein [:mcsmurf]]

Comment on attachment 674632 [details] [diff] [review]
Patch

Robert: Can you maybe take a quick look at this? See Comment 2 for how I created this patch. Changing the version info in the res editor was quite easy and the patch works as expected.

Comment 5

[Frank Wein [:mcsmurf]]

BTW: For additional security I uploaded the new 7zSD.sfx file to http://virusscan.jotti.org to check that no (known) virus or anything sneaked into the binary file/patch (don't worry, I have no virus or trojan on my PC afaik :).

Comment 6

[Robert Strong (they/them - no direct email)]

Comment on attachment 674632 [details] [diff] [review]
Patch

Should be fine though there are differences due to not using reshacker. For padding the tool used repeated PADDINGXX.

Comment 7

[Frank Wein [:mcsmurf]]

Pushed to comm-central (and used a virus scanner before check-in to make sure the file was still "clean"): https://hg.mozilla.org/comm-central/rev/78761068181f

Comment 8

[Frank Wein [:mcsmurf]]

Comment on attachment 674632 [details] [diff] [review]
Patch

[Approval Request Comment]
Regression caused by (bug #): -
User impact if declined: Escalation of privileges attack. A lower integrity process can become a high integrity process by placing a dll with arbitrary code by the given name in the downloads folder and waiting for the SeaMonkey installer to be executed.
Testing completed (on m-c, etc.): This binary patch has so far only received testing by people who build their own SeaMonkey comm-central build (due to broken tinderbox). But the patch also been tested in the Firefox installer as an almost identical patch landed in Bug 792106. The only difference are two version strings (replacing Firefox with SeaMonkey) and a different padding method (Windows executable files
Risk to taking this patch (and alternatives if risky): This patch affects the Windows installer only, should be low risk
String changes made by this patch: There is a minimum version required error inside the 7zip stub which cannot be localized at this time.
(comment is based on the Firefox approval request :-)

Comment 9

[Frank Wein [:mcsmurf]]

Forgot to finish this sentence in Comment 8
...and a different padding method (Windows PE format uses padding in the file format). The different padding method should not matter at all.

Comment 10

[Frank Wein [:mcsmurf]]

Pushed to comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/d09004ab8735
Pushed to comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/3a6e41a15ca2

Comment 11

[Frank Wein [:mcsmurf]]

Reopening based on Bug 792106

Comment 12

[Frank Wein [:mcsmurf]]

Attachment: Patch for cryptbase.dll issue

I used the same steps as in Comment 2 to create this binary patch. Carrying forward the r+ from Bug 792106 as the patch was already reviewed there and I only changed two strings with the resource editor.

Comment 13

[Justin Wood (:Callek)]

Comment on attachment 680047 [details] [diff] [review]
Patch for cryptbase.dll issue

Pre-Approving pending outcome of dep patch approval:

cryptbase SFX binary

(From other bug)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Since we've introduced 7zip wrapped installers
User impact if declined: escalation of privileges attack, a medium integrity process can become a high integrity process. (normal process can gain admin rights).
Testing completed (on m-c, etc.): I've tested locally and this just landed on m-c.
Risk to taking this patch (and alternatives if risky): Very low 
String or UUID changes made by this patch: none

Comment 14

[Frank Wein [:mcsmurf]]

Comment on attachment 680047 [details] [diff] [review]
Patch for cryptbase.dll issue

Pushed to comm-central: https://hg.mozilla.org/comm-central/rev/09c40eeb6fec
Pushed to comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/126cfcd448fa

Not pushed to beta as the patch in the original bug did not get approval either.

Comment 15

[Daniel Veditz [:dveditz]]

Seamonkey is not in-scope for the bug bounty.

Comment 16

[Frank Wein [:mcsmurf]]

Attachment: Patch for wmapi, cryptbase, shcore, and uxtheme

This patch copies the latest changes from mozilla-central to our version of the 7zSD.sfx. I used the same steps as in Comment 2 to create this binary patch. Carrying forward the r+ from Bug 792106 as the patch was already reviewed there and I only changed two strings with the resource editor.

Comment 17

[Frank Wein [:mcsmurf]]

Comment on attachment 697398 [details] [diff] [review]
Patch for wmapi, cryptbase, shcore, and uxtheme

Pushed to comm-central: https://hg.mozilla.org/comm-central/rev/849480ea2efc

Comment 18

[Al Billings [:abillings - ex-MoCo]]

This appears to be long fixed?

Comment 19

[Frank Wein [:mcsmurf]]

Yes, certainly fixed now. Just forgot to close the bug I think.

Comment 20

[Frank Wein [:mcsmurf]]

Though I guess the patch could have been pushed to the branches, too. But the security problem was not that critical.