Browse Source

Bug 2223: Some DLLs were not protected against hijacking

https://winscp.net/tracker/2223

Source commit: 7969689de8d933eb76d7646d8c867f4e1cbfa697
Martin Prikryl 2 years ago
parent
commit
323b110246

+ 0 - 3
source/Putty.cbproj

@@ -556,9 +556,6 @@
 		<CppCompile Include="putty\windows\utils\defaults.c">
 			<BuildOrder>140</BuildOrder>
 		</CppCompile>
-		<CppCompile Include="putty\windows\utils\dll_hijacking_protection.c">
-			<BuildOrder>141</BuildOrder>
-		</CppCompile>
 		<CppCompile Include="putty\windows\utils\escape_registry_key.c">
 			<BuildOrder>142</BuildOrder>
 		</CppCompile>

+ 0 - 1
source/WinSCP.cpp

@@ -37,7 +37,6 @@ WINAPI wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int)
     AppLog(L"Starting...");
 
     AddStartupSequence(L"M");
-    DllHijackingProtection();
     AppLogFmt(L"Process: %d", (GetCurrentProcessId()));
     AppLogFmt(L"Mouse: %s", (BooleanToEngStr(Mouse->MousePresent)));
     AppLogFmt(L"Mouse wheel: %s, msg: %d, scroll lines: %d", (BooleanToEngStr(Mouse->WheelPresent), int(Mouse->RegWheelMessage), Mouse->WheelScrollLines));

+ 0 - 5
source/core/PuttyIntf.cpp

@@ -1140,11 +1140,6 @@ UnicodeString __fastcall Sha256(const char * Data, size_t Size)
   return Result;
 }
 //---------------------------------------------------------------------------
-void __fastcall DllHijackingProtection()
-{
-  dll_hijacking_protection();
-}
-//---------------------------------------------------------------------------
 UnicodeString __fastcall ParseOpenSshPubLine(const UnicodeString & Line, const struct ssh_keyalg *& Algorithm)
 {
   UTF8String UtfLine = UTF8String(Line);

+ 0 - 2
source/core/PuttyTools.h

@@ -38,8 +38,6 @@ UnicodeString __fastcall GetPuTTYVersion();
 //---------------------------------------------------------------------------
 UnicodeString __fastcall Sha256(const char * Data, size_t Size);
 //---------------------------------------------------------------------------
-void __fastcall DllHijackingProtection();
-//---------------------------------------------------------------------------
 UnicodeString __fastcall ParseOpenSshPubLine(const UnicodeString & Line, const struct ssh_keyalg *& Algorithm);
 void ParseCertificatePublicKey(const UnicodeString & Str, RawByteString & PublicKey, UnicodeString & Fingerprint);
 bool IsCertificateValidityExpressionValid(

+ 13 - 0
source/packages/my/PasTools.pas

@@ -1130,10 +1130,23 @@ begin
   Result := (SysDarkTheme > 0);
 end;
 
+const
+  LOAD_LIBRARY_SEARCH_SYSTEM32 = $00000800;
+  LOAD_LIBRARY_SEARCH_USER_DIRS = $00000400;
+
 var
   Lib: THandle;
   OSVersionInfo: TOSVersionInfoEx;
+  SetDefaultDllDirectories: function(DirectoryFlags: DWORD): BOOL; stdcall;
 initialization
+  // Translated from PuTTY's dll_hijacking_protection().
+  // Inno Setup does not use LOAD_LIBRARY_SEARCH_USER_DIRS and falls back to SetDllDirectory.
+  Lib := LoadLibrary(kernel32);
+  SetDefaultDllDirectories := GetProcAddress(Lib, 'SetDefaultDllDirectories');
+  if Assigned(SetDefaultDllDirectories) then
+  begin
+    SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32 or LOAD_LIBRARY_SEARCH_USER_DIRS);
+  end;
   Lib := LoadLibrary('shcore');
   if Lib <> 0 then
   begin

+ 0 - 43
source/putty/windows/utils/dll_hijacking_protection.c

@@ -1,43 +0,0 @@
-/*
- * If the OS provides it, call SetDefaultDllDirectories() to prevent
- * DLLs from being loaded from the directory containing our own
- * binary, and instead only load from system32.
- *
- * This is a protection against hijacking attacks, if someone runs
- * PuTTY directly from their web browser's download directory having
- * previously been enticed into clicking on an unwise link that
- * downloaded a malicious DLL to the same directory under one of
- * various magic names that seem to be things that standard Windows
- * DLLs delegate to.
- *
- * It shouldn't break deliberate loading of user-provided DLLs such as
- * GSSAPI providers, because those are specified by their full
- * pathname by the user-provided configuration.
- */
-
-#include "putty.h"
-
-void dll_hijacking_protection(void)
-{
-    static HMODULE kernel32_module;
-    DECL_WINDOWS_FUNCTION(static, BOOL, SetDefaultDllDirectories, (DWORD));
-
-    if (!kernel32_module) {
-        kernel32_module = load_system32_dll("kernel32.dll");
-#if !HAVE_SETDEFAULTDLLDIRECTORIES
-        /* For older Visual Studio, this function isn't available in
-         * the header files to type-check */
-        GET_WINDOWS_FUNCTION_NO_TYPECHECK(
-            kernel32_module, SetDefaultDllDirectories);
-#else
-        GET_WINDOWS_FUNCTION(kernel32_module, SetDefaultDllDirectories);
-#endif
-    }
-
-    if (p_SetDefaultDllDirectories) {
-        /* LOAD_LIBRARY_SEARCH_SYSTEM32 and explicitly specified
-         * directories only */
-        p_SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32 |
-                                   LOAD_LIBRARY_SEARCH_USER_DIRS);
-    }
-}