-
-
Notifications
You must be signed in to change notification settings - Fork 309
Set SFX dependent load flags to only search System32 #770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Actually, 7-Zip mainline has some infrastructure to workaround the issue, read 'NanaZip.Core\SevenZip\C\DllSecur.c' for more information. Also, other parts of NanaZip also care about the DLL hijack mitigations, which should not have more issues. But your PR will enhance the DLL hijack-related mitigation, for example, between the process creation and enabling the policy at the start. (I have not cared for that before because I think it would be enough. DISM++ cares that via using a customized delay load handler, and uses delay load for non kernel32.dll dlls.) I hope you can add the linker mitigation options to all NanaZip exe/dll components, which will be better for all NanaZip 6.0 currently-supported Windows versions. Kenji Mouri |
|
Extra notes: If you want to know why NanaZip uses bcrypt.dll without the DLL hijack issues. Read https://github.com/ProjectMile/Mile.Windows.Helpers/blob/c026b0bdca0ed5ff39522b8253e3a13e3fd84857/Mile.Helpers/Mile.Helpers.Base.cpp#L388 for more information. (Because NanaZip loads K7Pal and NanaZip.Codecs dynamically with this function.) Kenji Mouri |
|
I know that defenses against DLL hijacking are already in 7-Zip and NanaZip. But these concern runtime linking by LoadLibrary(), not load-time linking during startup. Without this patch, load-time linking of the SFX could be influenced by using DLL redirection. For example, I've prepared 2 SFX files "safe.exe" and "vulnerable.exe", each with an empty "*.exe.local" file and a fake uxtheme.dll in the same folder. The vulnerable SFX looks for the local uxtheme.dll and thus fails to start up:
Whereas safe.exe with the dependent load flag starts up normally:
This is mainly a concern for SFX executables, which may run in unsafe locations (downloads folder, temp folders), but I don't think the same risk exists in other NanaZip executables. Therefore, for compat purposes (in case users may want to intentionally hijack DLLs), I've decided to only apply the load flag to SFX for now. |
|
I agree with you, except that "don't think the same risk exists in other NanaZip executables". It has a similar risk because NanaZip has a Classic flavor, which is designed as the portable version that is used in Windows PE/RE, Server Core, and Wine. Also, I think the compatibility work should be done by NanaZip itself instead of third-party extra magic things. This is why I hope to introduce this feature to all NanaZip executables. Also, maybe some guys want to do some experiments about alternative deployment ways for NanaZip Modern flavor (for example, they may hope to use NanaZip Modern flavor as portable versions to integrate to their Windows 11 PE instances, about recent years, some guys have made Windows 11 Version 23H2 or later PE which can use XAML start menu and settings app.), which can reduce more possible risks for these users. Kenji Mouri |
|
NanaZip Classic/Modern will run in its own folder most of the time, rather than scattered inside an uncontrolled downloads folder, which is why I don't think they have the same risk. But I'm not opposed to adding this to all executables. What's the best place to add this linker flag, |
|
I think it's good if you can add this flag to "NanaZip.Shared.Mitigations.props" with some comments, which provide some reference links. Kenji Mouri |
|
After testing, turns out this won't work without further changes due to static imports of NanaZip.Modern.dll. And the dynamic loading of NanaZip.Core/Codecs.dll means that the load flag is useless in this scenario anyway. So I don't see a way to extend it beyond SFX executables for now. |
|
OK. But it seems you can add a comment to notice with a reference link like "pbatard/rufus#2701 (comment)" you have provided. Because I'm afraid some future guys may remove that if without some notice. Kenji Mouri |
|
Also, what about helping me add some NanaZip's mitigation implementations to https://github.com/M2Team/NanaBox after I merge this PR (after you add the comments I will merge that). I can do that but I think you should have a chance to leave your footprint. Kenji Mouri |
Unlike the 7-Zip SFX, NanaZip SFX makes use of DLL imports that don't belong to KnownDlls (e.g. bcrypt.dll and uxtheme.dll). It is therefore vulnerable to a DLL planting attack if DLL redirection could be successfully triggered. Set the LOAD_LIBRARY_SEARCH_SYSTEM32 dependent load flag on Release builds to prevent this problem on Windows 10 build 14393 and later. Signed-off-by: Tu Dinh <[email protected]>


Unlike the 7-Zip SFX, NanaZip SFX makes use of DLL imports that don't belong to KnownDlls (e.g. bcrypt.dll and uxtheme.dll). It is therefore vulnerable to a DLL planting attack if DLL redirection could be successfully triggered.
Set the LOAD_LIBRARY_SEARCH_SYSTEM32 dependent load flag on Release builds to prevent this problem on Windows 10 build 14393 and later.
There are some caveats as described in this comment here at pbatard/rufus, but I couldn't trigger them in the SFX build. (Is it because Rufus is trying to load these DLLs dynamically, which isn't covered by this load flag?)