Closed Bug 804979 Opened 7 years ago Closed 7 years ago

DLL Hijacking - Seamonkey installer

Categories

(SeaMonkey :: Installer, defect, major)

x86_64
Windows 7
defect
Not set
major

Tracking

(firefox-esr17 unaffected, b2g18 unaffected, seamonkey2.14 affected, seamonkey2.15 affected, seamonkey2.16 fixed)

RESOLVED FIXED
seamonkey2.16
Tracking Status
firefox-esr17 --- unaffected
b2g18 --- unaffected
seamonkey2.14 --- affected
seamonkey2.15 --- affected
seamonkey2.16 --- fixed

People

(Reporter: mcsmurf, Assigned: mcsmurf)

References

Details

(Keywords: csectype-priv-escalation, sec-high)

Attachments

(3 files)

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.
Summary: DLL Hijacking - Firefox installer → DLL Hijacking - Seamonkey installer
Attached patch PatchSplinter Review
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 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.
Assignee: installer → bugzilla
Status: NEW → ASSIGNED
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.
Attachment #674632 - Flags: review?(robert.bugzilla)
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 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.
Attachment #674632 - Flags: review?(robert.bugzilla) → review+
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
Target Milestone: --- → seamonkey2.16
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 :-)
Attachment #674632 - Flags: approval-comm-beta?
Attachment #674632 - Flags: approval-comm-aurora?
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.
Attachment #674632 - Flags: approval-comm-beta?
Attachment #674632 - Flags: approval-comm-beta+
Attachment #674632 - Flags: approval-comm-aurora?
Attachment #674632 - Flags: approval-comm-aurora+
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Reopening based on Bug 792106
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Attachment #680047 - Flags: review+
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
Attachment #680047 - Flags: approval-comm-beta+
Attachment #680047 - Flags: approval-comm-aurora+
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.
Attachment #680047 - Flags: approval-comm-beta+
Seamonkey is not in-scope for the bug bounty.
Flags: sec-bounty-
Depends on: 811557
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.
Attachment #697398 - Flags: review+
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
This appears to be long fixed?
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Yes, certainly fixed now. Just forgot to close the bug I think.
Though I guess the patch could have been pushed to the branches, too. But the security problem was not that critical.
Group: core-security
You need to log in before you can comment on or make changes to this bug.