Closed Bug 806814 Opened 9 years ago Closed 8 years ago

We should call SetDllDirectory("") in our NSIS installers as a precaution

Categories

(Thunderbird :: Installer, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(thunderbird21+ fixed, thunderbird22+ fixed, thunderbird-esr1721+ fixed)

RESOLVED FIXED
Thunderbird 23.0
Tracking Status
thunderbird21 + fixed
thunderbird22 + fixed
thunderbird-esr17 21+ fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: sec-moderate)

Attachments

(1 file)

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

There is no known DLL injection call that would be fixed by calling SetDllDirectory(""), but it would be good practice to do so. 

SetDllDirectory("") helps only with LoadLibrary calls, so this isn't a blanket fix for all DLL injection attacks to the installer.  In particular it doesn't help with implicitly linked DLLs. 

Leaving as security-sensitive for now in case there is a problem I don't know about.

See also bug 792106
Attached patch The fixSplinter Review
Directly ported from bug 801853.

Robert, would you mind doing the review of this as you reviewed that patch?
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #735080 - Flags: review?(robert.bugzilla)
Attachment #735080 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 735080 [details] [diff] [review]
The fix

[Triage Comment]
As Thunderbird trunk is currently closed, I've landed this directly on beta as we're just about to build our one and only beta for this cycle. I'll leave open to land on trunk & aurora.
Attachment #735080 - Flags: approval-comm-beta+
Attachment #735080 - Flags: approval-comm-aurora?
https://hg.mozilla.org/comm-central/rev/10d2778de689
https://hg.mozilla.org/releases/comm-aurora/rev/9c12a1bd9692
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0
Attachment #735080 - Flags: approval-comm-aurora? → approval-comm-aurora+
Comment on attachment 735080 [details] [diff] [review]
The fix

[Triage Comment]
a=me for ESR (although this is only moderate, 17 is our main releases at the moment).
Attachment #735080 - Flags: approval-comm-esr17+
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.