Package: nsis
Version: 2.46-10
Installers generated by NSIS 2.46 are vulnerable to attacks that can lead to
code execution and privilege escalation (if the installer is running with
elevated privileges).
This has been reported to us at Gpg4win (www.gpg4win.org) which is built under
Debian GNU/Linux. We saw no other option to mitigate the attacks then to patch
our version of NSIS.
We've also reported this upstream today:
https://sourceforge.net/p/nsis/bugs/1125/
Background: Windows loads Libraries that are not "Known DLL's" from the
directory of the executable first. As NSIS uses direct loading though
LoadLibrary / LoadLibraryEx system calls, and links a not "Well Known" library
(version.dll) placing Libraries with standard names like shfolder.dll or
version.dll in the same Folder as an NSIS Installer (usually the Downloads
folder) will load those libraries in the context of the installer. An attacker
could cause these Libraries to be executed in the context of the installer.
This is especially problematic for signed Installers and Installers that
require Elevated (Administrator) Privileges for installation. As this bypasses
the signature validation and can be used for a privilege escalation.
Additionally NSIS uses an insecure temporary directory that can be modified
with normal User access rights in case of an elevated installation. This can
be used to modify plugins in that directory which then will be loaded with
higher privileges. There is also a temporary file race on uninstallation where
the uninstaller is copied into a temporary directory and afterwards executed
with elevated privileges.
More details and descriptions about how we mitigate these attacks are
available in the NSIS bug report.
Attached are the patches we are planning to use with NSIS to prepare our
next gpg4win release. They can be applied in the Order of their names to the
debian version of NSIS and compile on wheezy and jessie.
Regards,
Andre
--
Andre Heinecke | ++49-541-335083-262 | http://www.intevation.de/
Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998
Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
--- a/Source/exehead/util.c
+++ b/Source/exehead/util.c
@@ -942,12 +942,54 @@
{"SHFOLDER", "SHGetFolderPathA"}
};
+#ifndef LOAD_LIBRARY_SEARCH_SYSTEM32
+#define LOAD_LIBRARY_SEARCH_SYSTEM32 0x00000800
+#endif
+
+#ifndef LOAD_LIBRARY_SEARCH_USER_DIRS
+#define LOAD_LIBRARY_SEARCH_USER_DIRS 0x00000400
+#endif
+
+// Check if the system supports LoadLibraryEx flags to
+// restrict the search order. This migitates current
+// directory attacks.
+void NSISCALL RestrictLibrarySearch()
+{
+ typedef BOOL (WINAPI *sddd_t)(DWORD DirectoryFlags);
+ sddd_t sddd;
+ HMODULE hModule = GetModuleHandle("KERNEL32");
+ if (!hModule)
+ {
+ /* KERNEL32.dll is one of the few Known libraries
+ on Windows so this is safe to do */
+ hModule = LoadLibrary("KERNEL32");
+ }
+
+ sddd = (sddd_t) GetProcAddress (hModule, "SetDefaultDllDirectories");
+
+ if (!sddd)
+ {
+ // SetDefaultDllDirectories not found this means we are either
+ // on an unmaintained Windows or on a windows that does
+ // not have KB2533623 installed.
+ log_printf("SetDefaultDllDirectories failed.");
+ return;
+ }
+
+ /* Users may use AddDllDirectory (SEARCH_USER_DIRS) but otherwise
+ we only load from System32. No installer should depend on the
+ insecure behavior of loading libraries from the directory in which
+ the installer is placed. This prevents DLL search order attacks
+ (CAPEC-471) against the current directory. */
+ sddd(LOAD_LIBRARY_SEARCH_USER_DIRS | LOAD_LIBRARY_SEARCH_SYSTEM32);
+}
+
void * NSISCALL myGetProcAddress(const enum myGetProcAddressFunctions func)
{
const char *dll = MGA_FUNCS[func].dll;
HMODULE hModule = GetModuleHandle(dll);
if (!hModule)
- hModule = LoadLibrary(dll);
+ hModule = LoadLibraryEx(dll, NULL, 0);
if (!hModule)
return NULL;
--- a/Source/exehead/util.h
+++ b/Source/exehead/util.h
@@ -117,6 +118,10 @@
void * NSISCALL myGetProcAddress(const enum myGetProcAddressFunctions func);
void NSISCALL MessageLoop(UINT uCheckedMsg);
+// Restrict the LoadLibraryEx library search folder to mitigate current
+// directory attacks.
+void NSISCALL RestrictLibrarySearch();
+
// Turn a pair of chars into a word
// Turn four chars into a dword
#ifdef __BIG_ENDIAN__ // Not very likely, but, still...
--- a/Contrib/System/Source/System.c
+++ b/Contrib/System/Source/System.c
@@ -703,7 +703,7 @@
{
// Get DLL address
if ((proc->Dll = GetModuleHandle(proc->DllName)) == NULL)
- if ((proc->Dll = LoadLibrary(proc->DllName)) == NULL)
+ if ((proc->Dll = LoadLibraryEx(proc->DllName, NULL, 0)) == NULL)
{
proc->ProcResult = PR_ERROR;
break;
--- a/Source/exehead/Ui.c
+++ b/Source/exehead/Ui.c
@@ -382,9 +382,9 @@
static const char riched32[]="RichEd32";
static const char richedit20a[]="RichEdit20A";
static const char richedit[]="RichEdit";
- if (!LoadLibrary(riched20))
+ if (!LoadLibraryEx(riched20, NULL, 0))
{
- LoadLibrary(riched32);
+ LoadLibraryEx(riched32, NULL, 0);
}
// make richedit20a point to RICHEDIT
--- a/Contrib/Dialer/dialer.c
+++ b/Contrib/Dialer/dialer.c
@@ -17,7 +17,7 @@
HMODULE hWinInet = NULL;
FARPROC GetWinInetFunc(char *func) {
- hWinInet = LoadLibrary("wininet.dll");
+ hWinInet = LoadLibraryEx("wininet.dll", NULL, 0);
if (hWinInet)
return GetProcAddress(hWinInet, func);
return NULL;
--- a/Contrib/UIs/ui.c
+++ b/Contrib/UIs/ui.c
@@ -85,7 +85,7 @@
{
InitCommonControls();
- LoadLibrary("RichEd32.dll");
+ LoadLibraryEx("RichEd32.dll", NULL, 0);
g_hInstance = GetModuleHandle(0);
--- a/Source/exehead/Main.c
+++ b/Source/exehead/Main.c
@@ -66,6 +66,8 @@
char seekchar=' ';
char *cmdline;
+ RestrictLibrarySearch();
+
InitCommonControls();
SetErrorMode(SEM_NOOPENFILEERRORBOX | SEM_FAILCRITICALERRORS);
--- a/Source/exehead/Main.c
+++ b/Source/exehead/Main.c
@@ -225,7 +225,7 @@
if (!lstrcmpi(state_temp_dir,state_exe_directory))
goto end;
- CreateDirectory(state_temp_dir,NULL);
+ CreateRestrictedDirectory(state_temp_dir);
SetCurrentDirectory(state_temp_dir);
if (!(*state_install_directory))
--- a/Source/exehead/util.c
+++ b/Source/exehead/util.c
@@ -15,6 +15,8 @@
*/
#include "../Platform.h"
+#include <accctrl.h>
+#include <aclapi.h>
#include <shellapi.h>
#include "util.h"
#include "state.h"
@@ -1002,3 +1004,167 @@
while (PeekMessage(&msg, NULL, uCheckedMsg, uCheckedMsg, PM_REMOVE))
DispatchMessage(&msg);
}
+
+// Check if the current process is elevated.
+static BOOL IsElevated()
+{
+ HANDLE hToken = NULL;
+ BOOL ret = FALSE;
+ if (OpenProcessToken (GetCurrentProcess(), TOKEN_QUERY, &hToken))
+ {
+ DWORD elevation;
+ DWORD cbSize = sizeof (DWORD);
+ if (GetTokenInformation (hToken, TokenElevation, &elevation,
+ sizeof (TokenElevation), &cbSize))
+ ret = elevation;
+ }
+ if (hToken)
+ CloseHandle (hToken);
+
+ return ret;
+}
+
+BOOL NSISCALL CreateRestrictedDirectory(char *path)
+{
+ BOOL retval = FALSE;
+ PSID everyone_SID = NULL,
+ admin_SID = NULL;
+ PACL access_control_list = NULL;
+ PSECURITY_DESCRIPTOR descriptor = NULL;
+ EXPLICIT_ACCESS explicit_access[2];
+ SID_IDENTIFIER_AUTHORITY world_identifier = {SECURITY_WORLD_SID_AUTHORITY},
+ admin_identifier = {SECURITY_NT_AUTHORITY};
+ SECURITY_ATTRIBUTES security_attributes;
+
+ ZeroMemory(&security_attributes, sizeof(security_attributes));
+ ZeroMemory(&explicit_access, 2 * sizeof(EXPLICIT_ACCESS));
+
+ // If not elevated we are already done.
+ if (!IsElevated())
+ return CreateDirectory(path, NULL);
+
+ // Create a well-known SID for the Everyone group.
+ if(!AllocateAndInitializeSid(&world_identifier, // top-level identifier
+ 1, // subauthorties count
+ SECURITY_WORLD_RID, // Only one authority
+ 0, 0, 0, 0, 0, 0, 0, // No other authorities
+ &everyone_SID))
+ {
+ log_printf("CreateRestrictedDirectory: Failed to allocate w sid.\n");
+ return FALSE;
+ }
+
+ // Initialize the first EXPLICIT_ACCESS structure for an ACE.
+ // to allow everyone read access. Probably uneccessary but there
+ // may be installers that rely on reading the folder from unprivileged
+ // code.
+ explicit_access[0].grfAccessPermissions = GENERIC_READ; // Give read access
+ explicit_access[0].grfAccessMode = SET_ACCESS; // Overwrite other access
+ explicit_access[0].grfInheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT;
+ explicit_access[0].Trustee.TrusteeForm = TRUSTEE_IS_SID;
+ explicit_access[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP;
+ explicit_access[0].Trustee.ptstrName = (LPTSTR) everyone_SID;
+
+ // Create the SID for the BUILTIN\Administrators group.
+ if(!AllocateAndInitializeSid(&admin_identifier,
+ 2,
+ SECURITY_BUILTIN_DOMAIN_RID, /*BUILTIN\ */
+ DOMAIN_ALIAS_RID_ADMINS, /*\Administrators */
+ 0, 0, 0, 0, 0, 0, /* No other */
+ &admin_SID))
+ {
+ log_printf("CreateRestrictedDirectory: Failed to allocate adm sid.");
+ goto done;
+ }
+
+ /// explicit_access[1] grants admins full rights for the folder
+ explicit_access[1].grfAccessPermissions = GENERIC_ALL;
+ explicit_access[1].grfAccessMode = SET_ACCESS;
+ explicit_access[1].grfInheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT;
+ explicit_access[1].Trustee.TrusteeForm = TRUSTEE_IS_SID;
+ explicit_access[1].Trustee.TrusteeType = TRUSTEE_IS_GROUP;
+ explicit_access[1].Trustee.ptstrName = (LPTSTR) admin_SID;
+
+ // Set up the ACL structure.
+ if (ERROR_SUCCESS != SetEntriesInAcl(2, explicit_access, NULL,
+ &access_control_list))
+ {
+ log_printf("CreateRestrictedDirectory: Failed to set up Acl.");
+ goto done;
+ }
+
+ // Initialize the security descriptor.
+ descriptor = (PSECURITY_DESCRIPTOR) LocalAlloc(LPTR,
+ SECURITY_DESCRIPTOR_MIN_LENGTH);
+ if (descriptor == NULL)
+ {
+ log_printf("CreateRestrictedDirectory: Failed to alloc descriptor.");
+ goto done;
+ }
+
+ if (!InitializeSecurityDescriptor(descriptor,
+ SECURITY_DESCRIPTOR_REVISION))
+ {
+ log_printf("CreateRestrictedDirectory: Failed to init descriptor.");
+ goto done;
+ }
+
+ // Now we add the ACL to the the descriptor
+ if (!SetSecurityDescriptorDacl(descriptor,
+ TRUE, /* bDaclPresent flag */
+ access_control_list,
+ FALSE)) /* not a default DACL */
+ {
+ log_printf("CreateRestrictedDirectory: Failed to set ACL.");
+ goto done;
+ }
+
+ // Finally set up the security attributes structure
+ security_attributes.nLength = sizeof (SECURITY_ATTRIBUTES);
+ security_attributes.lpSecurityDescriptor = descriptor;
+ security_attributes.bInheritHandle = FALSE;
+
+ if (!CreateDirectory(path, &security_attributes))
+ {
+ // If the directory already exists we try to restrict access
+ DWORD err = GetLastError();
+ if (err == ERROR_ALREADY_EXISTS)
+ {
+ err = SetNamedSecurityInfo (path,
+ SE_FILE_OBJECT,
+ DACL_SECURITY_INFORMATION |
+ OWNER_SECURITY_INFORMATION |
+ GROUP_SECURITY_INFORMATION,
+ admin_SID, /* owner */
+ admin_SID, /* group */
+ access_control_list, /* the dacl */
+ NULL);
+ if (err != ERROR_SUCCESS)
+ {
+ log_printf2("CreateRestrictedDirectory Failed to set sec info: %lu",
+ err);
+ goto done;
+ }
+ }
+ else
+ {
+ log_printf2("CreateRestrictedDirectory Failed: %lu", err);
+ goto done;
+ }
+ }
+
+ retval = TRUE;
+
+done:
+
+ if (everyone_SID)
+ FreeSid(everyone_SID);
+ if (admin_SID)
+ FreeSid(admin_SID);
+ if (access_control_list)
+ LocalFree(access_control_list);
+ if (descriptor)
+ LocalFree(descriptor);
+
+ return retval;
+}
--- a/Source/exehead/util.h
+++ b/Source/exehead/util.h
@@ -121,6 +121,12 @@
// directory attacks.
void NSISCALL RestrictLibrarySearch();
+// Create a directory that is only writable by administrators if
+// the installer is running with elevated privilges normal CreateDirectory
+// otherwise
+// returns true on success
+BOOL NSISCALL CreateRestrictedDirectory(char * path);
+
// Turn a pair of chars into a word
// Turn four chars into a dword
#ifdef __BIG_ENDIAN__ // Not very likely, but, still...
--- a/Source/build.cpp
+++ b/Source/build.cpp
@@ -3317,7 +3317,9 @@
ret=add_entry_direct(EW_DELETEFILE, zero_offset, DEL_SIMPLE);
if (ret != PS_OK) return ret;
// CraeteDirectory $0 - a dir instead of that temp file
- ret=add_entry_direct(EW_CREATEDIR, zero_offset);
+ // Restrict the directory if we are elevated to prevent privilege escalation
+ // attacks.
+ ret=add_entry_direct(EW_CREATEDIR_RESTRICT, zero_offset);
if (ret != PS_OK) return ret;
// IfErrors Initialize_____Plugins_error - detect errors
ret=add_entry_direct(EW_IFFLAG, ns_label.add("Initialize_____Plugins_error",0), 0, FLAG_OFFSET(exec_error));
--- a/Source/exehead/exec.c
+++ b/Source/exehead/exec.c
@@ -311,6 +311,7 @@
}
}
break;
+ case EW_CREATEDIR_RESTRICT:
case EW_CREATEDIR: {
char *buf1=GetStringFromParm(-0x10);
log_printf3("CreateDirectory: \"%s\" (%d)",buf1,parm1);
@@ -324,7 +325,15 @@
p = findchar(p, '\\');
c = *p;
*p = 0;
- if (!CreateDirectory(buf1, NULL))
+ BOOL err;
+ // If restricted access is requested create the last directory
+ // restricted.
+ if (!c && which == EW_CREATEDIR_RESTRICT)
+ err = CreateRestrictedDirectory (buf1);
+ else
+ err = CreateDirectory(buf1, NULL);
+
+ if (!err)
{
if (GetLastError() != ERROR_ALREADY_EXISTS)
{
--- a/Source/exehead/fileform.h
+++ b/Source/exehead/fileform.h
@@ -59,6 +59,7 @@
EW_CHDETAILSVIEW, // SetDetailsView: 2 [listaction,buttonaction]
EW_SETFILEATTRIBUTES, // SetFileAttributes: 2 [filename, attributes]
EW_CREATEDIR, // Create directory: 2, [path, ?update$INSTDIR]
+ EW_CREATEDIR_RESTRICT,// Like Create directory but access restricted if process is elvated: 2, [path, ?update$INSTDIR]
EW_IFFILEEXISTS, // IfFileExists: 3, [file name, jump amount if exists, jump amount if not exists]
EW_SETFLAG, // Sets a flag: 2 [id, data]
EW_IFFLAG, // If a flag: 4 [on, off, id, new value mask]
--- a/Source/exehead/SConscript
+++ b/Source/exehead/SConscript
@@ -43,7 +43,6 @@
comdlg32
comctl32
ole32
- version
uuid
""")
--- a/Source/exehead/exec.c
+++ b/Source/exehead/exec.c
@@ -934,7 +934,7 @@
VS_FIXEDFILEINFO *pvsf1;
DWORD d;
char *buf1=GetStringFromParm(-0x12);
- s1=GetFileVersionInfoSize(buf1,&d);
+ s1=myGetFileVersionInfoSizeA(buf1,&d);
*lowout=*highout=0;
exec_error++;
if (s1)
@@ -944,7 +944,7 @@
if (b1)
{
UINT uLen;
- if (GetFileVersionInfo(buf1,0,s1,b1) && VerQueryValue(b1,"\\",(void*)&pvsf1,&uLen))
+ if (myGetFileVersionInfoA(buf1,0,s1,b1) && myVerQueryValueA(b1,"\\",(void*)&pvsf1,&uLen))
{
myitoa(highout,pvsf1->dwFileVersionMS);
myitoa(lowout,pvsf1->dwFileVersionLS);
--- a/Source/exehead/util.c
+++ b/Source/exehead/util.c
@@ -941,7 +941,10 @@
{"ADVAPI32", "AdjustTokenPrivileges"},
{"KERNEL32", "GetUserDefaultUILanguage"},
{"SHLWAPI", "SHAutoComplete"},
- {"SHFOLDER", "SHGetFolderPathA"}
+ {"SHFOLDER", "SHGetFolderPathA"},
+ {"VERSION", "GetFileVersionInfoSizeA"},
+ {"VERSION", "GetFileVersionInfoA"},
+ {"VERSION", "VerQueryValueA"}
};
#ifndef LOAD_LIBRARY_SEARCH_SYSTEM32
@@ -1168,3 +1171,42 @@
return retval;
}
+
+BOOL NSISCALL myGetFileVersionInfoA(LPCSTR lpcstrFilename, DWORD dwHandle,
+ DWORD dwLen, LPVOID lpData)
+{
+ typedef BOOL (WINAPI * GetFileVersionInfoAPtr)(LPCSTR, DWORD, DWORD, LPVOID);
+ GetFileVersionInfoAPtr GFVI = (GetFileVersionInfoAPtr)
+ myGetProcAddress(MGA_GetFileVersionInfoA);
+
+ if (!GFVI)
+ return FALSE;
+
+ return GFVI (lpcstrFilename, dwHandle, dwLen, lpData);
+}
+
+DWORD NSISCALL myGetFileVersionInfoSizeA(LPCSTR lpcstrFilename,
+ LPDWORD lpdwHandle)
+{
+ typedef DWORD (WINAPI * GetFileVersionInfoSizeAPtr)(LPCSTR, LPDWORD);
+ GetFileVersionInfoSizeAPtr GFVIS = (GetFileVersionInfoSizeAPtr)
+ myGetProcAddress(MGA_GetFileVersionInfoSizeA);
+
+ if (!GFVIS)
+ return 0;
+
+ return GFVIS (lpcstrFilename, lpdwHandle);
+}
+
+BOOL NSISCALL myVerQueryValueA(LPCVOID pBlock, LPCSTR lpSubBlock,
+ LPVOID *lplpBuffer, PUINT puLen)
+{
+ typedef DWORD (WINAPI * VerQueryValueAPtr)(LPCVOID, LPCSTR, LPVOID*, PUINT);
+ VerQueryValueAPtr VQV = (VerQueryValueAPtr)
+ myGetProcAddress(MGA_VerQueryValueA);
+
+ if (!VQV)
+ return FALSE;
+
+ return VQV(pBlock, lpSubBlock, lplpBuffer, puLen);
+}
--- a/Source/exehead/util.h
+++ b/Source/exehead/util.h
@@ -111,7 +111,10 @@
MGA_AdjustTokenPrivileges,
MGA_GetUserDefaultUILanguage,
MGA_SHAutoComplete,
- MGA_SHGetFolderPathA
+ MGA_SHGetFolderPathA,
+ MGA_GetFileVersionInfoSizeA,
+ MGA_GetFileVersionInfoA,
+ MGA_VerQueryValueA
};
void * NSISCALL myGetProcAddress(const enum myGetProcAddressFunctions func);
@@ -127,6 +130,17 @@
// returns true on success
BOOL NSISCALL CreateRestrictedDirectory(char * path);
+#ifdef _WIN32
+// Wrapper for functions from version.dll to avoid implicit linkage
+// as version.dll is not a known library on windows and thus insecure.
+BOOL NSISCALL myGetFileVersionInfoA(LPCSTR lpcstrFilename, DWORD dwHandle,
+ DWORD dwLen, LPVOID lpData);
+DWORD NSISCALL myGetFileVersionInfoSizeA(LPCSTR lpcstrFilename,
+ LPDWORD lpdwHandle);
+BOOL NSISCALL myVerQueryValueA(LPCVOID pBlock, LPCSTR lpSubBlock,
+ LPVOID *lplpBuffer, PUINT puLen);
+#endif
+
// Turn a pair of chars into a word
// Turn four chars into a dword
#ifdef __BIG_ENDIAN__ // Not very likely, but, still...
