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
(In reply to Brian R. Bondy [:bbondy] from comment #0) > There is no known DLL injection call that would be fixed by calling > SetDllDirectory(""), but it would be good practice to do so. The problem can come from the OS itself -- Some of the libraries will attempt to load other OS dlls to see if they're there, and have fallback code if they're not (e.g. Vista vs. Win7). This is, basically, where the dwmapi.dll problem comes from, and I'd lay odds that new stuff will be added in Win8 that will thereafter make Win7 and Vista users vulnerable to injections that are not seen today.
Agreed, in these implicitly loaded cases though, SetDllDirectoryW(L"") will not help.
Comment on attachment 671835 [details] [diff] [review] Path v1 Seems like you should add it to UninstallUnOnInitCommon as well. There is other code that runs before the common macros and I wonder if it wouldn't be better to run it in the associated .oninit, etc.
I think we can just remove the common.nsh changes and let other products port this fix into their own nsi files.
Comment on attachment 675389 [details] [diff] [review] Patch v2. (In reply to Brian R. Bondy [:bbondy] from comment #5) > Created attachment 675389 [details] [diff] [review] > Patch v2. > > I think we can just remove the common.nsh changes and let other products > port this fix into their own nsi files. If there is any concern about code running before this call then that would be the better way to go and r=me if you want to go with that instead.
Attachment #675389 - Flags: review?(robert.bugzilla) → review+
I'm not really concerned but I think that's the best way, so I'll go with the current patch minus common.nsh and r=you. Thanks!
Not a problem and I hope you have fun for a week without any stub craziness! :)
Same as last patch -common.nsh. Carrying forward r+. (In reply to Robert Strong [:rstrong] (vac 10/26-11/4) (do not email) from comment #8) > Not a problem and I hope you have fun for a week without any stub craziness! > :) fingers crossed :)
Target Milestone: --- → mozilla19
This doesn't meet ESR criteria (sec-moderate) so marking wontfix on esr17.
Marking this wontfix for b2g as well as it is sec-moderate. If needed,please nominate(tracking-b2g18) in case this would be more severe for b2g vs ESR to get this landed.
You need to log in before you can comment on or make changes to this bug.