* Emacs master, security concernes, ms-windows
@ 2017-09-14 7:58 Fabrice Popineau
2017-09-14 13:33 ` Óscar Fuentes
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Fabrice Popineau @ 2017-09-14 7:58 UTC (permalink / raw
To: Emacs developers
[-- Attachment #1.1: Type: text/plain, Size: 531 bytes --]
Since there seems to be a lot of concerns wrt to security,
I am submitting the attached patch.
The reason for this patch is to limit the search for dlls loaded at
runtime to the win32 system directory and/or the emacs application
directory.
In the current state, dlls can be picked up in any directory in the path.
Some one could fake one of these dlls (xpm, png, etc.) and use it for
mean reasons.
It is not bullet proof, but it levels up security and
many other projects have applied such a restriction.
Best regards,
Fabrice
[-- Attachment #1.2: Type: text/html, Size: 772 bytes --]
[-- Attachment #2: emacs-loadlibrary.diff --]
[-- Type: text/plain, Size: 15300 bytes --]
diff --git a/src/unexw32.c b/src/unexw32.c
index 5259b2a52b..10f720f734 100644
--- a/src/unexw32.c
+++ b/src/unexw32.c
@@ -772,7 +820,7 @@ unexec (const char *new_name, const char *old_name)
{
PIMAGE_DOS_HEADER dos_header;
PIMAGE_NT_HEADERS nt_header;
- HANDLE hImagehelp = LoadLibrary ("imagehlp.dll");
+ HANDLE hImagehelp = LoadLibraryEx ("imagehlp.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
DWORD headersum;
DWORD checksum;
diff --git a/src/w32.c b/src/w32.c
index f583d5e76c..a437d3fd83 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -565,7 +565,7 @@ open_process_token (HANDLE ProcessHandle,
if (g_b_init_open_process_token == 0)
{
g_b_init_open_process_token = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Open_Process_Token =
(OpenProcessToken_Proc) GetProcAddress (hm_advapi32, "OpenProcessToken");
}
@@ -597,7 +597,7 @@ get_token_information (HANDLE TokenHandle,
if (g_b_init_get_token_information == 0)
{
g_b_init_get_token_information = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Token_Information =
(GetTokenInformation_Proc) GetProcAddress (hm_advapi32, "GetTokenInformation");
}
@@ -633,7 +633,7 @@ lookup_account_sid (LPCTSTR lpSystemName,
if (g_b_init_lookup_account_sid == 0)
{
g_b_init_lookup_account_sid = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Lookup_Account_Sid =
(LookupAccountSid_Proc) GetProcAddress (hm_advapi32, LookupAccountSid_Name);
}
@@ -666,7 +666,7 @@ get_sid_sub_authority (PSID pSid, DWORD n)
if (g_b_init_get_sid_sub_authority == 0)
{
g_b_init_get_sid_sub_authority = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Sid_Sub_Authority =
(GetSidSubAuthority_Proc) GetProcAddress (
hm_advapi32, "GetSidSubAuthority");
@@ -691,7 +691,7 @@ get_sid_sub_authority_count (PSID pSid)
if (g_b_init_get_sid_sub_authority_count == 0)
{
g_b_init_get_sid_sub_authority_count = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Sid_Sub_Authority_Count =
(GetSidSubAuthorityCount_Proc) GetProcAddress (
hm_advapi32, "GetSidSubAuthorityCount");
@@ -722,7 +722,7 @@ get_security_info (HANDLE handle,
if (g_b_init_get_security_info == 0)
{
g_b_init_get_security_info = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Security_Info =
(GetSecurityInfo_Proc) GetProcAddress (
hm_advapi32, "GetSecurityInfo");
@@ -758,7 +758,7 @@ get_file_security (const char *lpFileName,
if (g_b_init_get_file_security_w == 0)
{
g_b_init_get_file_security_w = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_File_SecurityW =
(GetFileSecurityW_Proc) GetProcAddress (hm_advapi32,
"GetFileSecurityW");
@@ -780,7 +780,7 @@ get_file_security (const char *lpFileName,
if (g_b_init_get_file_security_a == 0)
{
g_b_init_get_file_security_a = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_File_SecurityA =
(GetFileSecurityA_Proc) GetProcAddress (hm_advapi32,
"GetFileSecurityA");
@@ -817,7 +817,7 @@ set_file_security (const char *lpFileName,
if (g_b_init_set_file_security_w == 0)
{
g_b_init_set_file_security_w = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Set_File_SecurityW =
(SetFileSecurityW_Proc) GetProcAddress (hm_advapi32,
"SetFileSecurityW");
@@ -838,7 +838,7 @@ set_file_security (const char *lpFileName,
if (g_b_init_set_file_security_a == 0)
{
g_b_init_set_file_security_a = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Set_File_SecurityA =
(SetFileSecurityA_Proc) GetProcAddress (hm_advapi32,
"SetFileSecurityA");
@@ -878,7 +878,7 @@ set_named_security_info (LPCTSTR lpObjectName,
if (g_b_init_set_named_security_info_w == 0)
{
g_b_init_set_named_security_info_w = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Set_Named_Security_InfoW =
(SetNamedSecurityInfoW_Proc) GetProcAddress (hm_advapi32,
"SetNamedSecurityInfoW");
@@ -900,7 +900,7 @@ set_named_security_info (LPCTSTR lpObjectName,
if (g_b_init_set_named_security_info_a == 0)
{
g_b_init_set_named_security_info_a = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Set_Named_Security_InfoA =
(SetNamedSecurityInfoA_Proc) GetProcAddress (hm_advapi32,
"SetNamedSecurityInfoA");
@@ -932,7 +932,7 @@ get_security_descriptor_owner (PSECURITY_DESCRIPTOR pSecurityDescriptor,
if (g_b_init_get_security_descriptor_owner == 0)
{
g_b_init_get_security_descriptor_owner = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Security_Descriptor_Owner =
(GetSecurityDescriptorOwner_Proc) GetProcAddress (
hm_advapi32, "GetSecurityDescriptorOwner");
@@ -961,7 +961,7 @@ get_security_descriptor_group (PSECURITY_DESCRIPTOR pSecurityDescriptor,
if (g_b_init_get_security_descriptor_group == 0)
{
g_b_init_get_security_descriptor_group = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Security_Descriptor_Group =
(GetSecurityDescriptorGroup_Proc) GetProcAddress (
hm_advapi32, "GetSecurityDescriptorGroup");
@@ -991,7 +991,7 @@ get_security_descriptor_dacl (PSECURITY_DESCRIPTOR pSecurityDescriptor,
if (g_b_init_get_security_descriptor_dacl == 0)
{
g_b_init_get_security_descriptor_dacl = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Security_Descriptor_Dacl =
(GetSecurityDescriptorDacl_Proc) GetProcAddress (
hm_advapi32, "GetSecurityDescriptorDacl");
@@ -1018,7 +1018,7 @@ is_valid_sid (PSID sid)
if (g_b_init_is_valid_sid == 0)
{
g_b_init_is_valid_sid = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Is_Valid_Sid =
(IsValidSid_Proc) GetProcAddress (
hm_advapi32, "IsValidSid");
@@ -1042,7 +1042,7 @@ equal_sid (PSID sid1, PSID sid2)
if (g_b_init_equal_sid == 0)
{
g_b_init_equal_sid = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Equal_Sid =
(EqualSid_Proc) GetProcAddress (
hm_advapi32, "EqualSid");
@@ -1066,7 +1066,7 @@ get_length_sid (PSID sid)
if (g_b_init_get_length_sid == 0)
{
g_b_init_get_length_sid = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Get_Length_Sid =
(GetLengthSid_Proc) GetProcAddress (
hm_advapi32, "GetLengthSid");
@@ -1090,7 +1090,7 @@ copy_sid (DWORD destlen, PSID dest, PSID src)
if (g_b_init_copy_sid == 0)
{
g_b_init_copy_sid = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Copy_Sid =
(CopySid_Proc) GetProcAddress (
hm_advapi32, "CopySid");
@@ -1362,7 +1362,7 @@ get_adapters_info (PIP_ADAPTER_INFO pAdapterInfo, PULONG pOutBufLen)
if (g_b_init_get_adapters_info == 0)
{
g_b_init_get_adapters_info = 1;
- hm_iphlpapi = LoadLibrary ("Iphlpapi.dll");
+ hm_iphlpapi = LoadLibraryEx ("Iphlpapi.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (hm_iphlpapi)
s_pfn_Get_Adapters_Info = (GetAdaptersInfo_Proc)
GetProcAddress (hm_iphlpapi, "GetAdaptersInfo");
@@ -6620,7 +6624,7 @@ open_thread_token (HANDLE ThreadHandle,
if (g_b_init_open_thread_token == 0)
{
g_b_init_open_thread_token = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Open_Thread_Token =
(OpenThreadToken_Proc) GetProcAddress (hm_advapi32, "OpenThreadToken");
}
@@ -6650,7 +6654,7 @@ impersonate_self (SECURITY_IMPERSONATION_LEVEL ImpersonationLevel)
if (g_b_init_impersonate_self == 0)
{
g_b_init_impersonate_self = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Impersonate_Self =
(ImpersonateSelf_Proc) GetProcAddress (hm_advapi32, "ImpersonateSelf");
}
@@ -6673,7 +6677,7 @@ revert_to_self (void)
if (g_b_init_revert_to_self == 0)
{
g_b_init_revert_to_self = 1;
- hm_advapi32 = LoadLibrary ("Advapi32.dll");
+ hm_advapi32 = LoadLibraryEx ("Advapi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_Revert_To_Self =
(RevertToSelf_Proc) GetProcAddress (hm_advapi32, "RevertToSelf");
}
@@ -6698,7 +6702,7 @@ get_process_memory_info (HANDLE h_proc,
if (g_b_init_get_process_memory_info == 0)
{
g_b_init_get_process_memory_info = 1;
- hm_psapi = LoadLibrary ("Psapi.dll");
+ hm_psapi = LoadLibraryEx ("Psapi.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (hm_psapi)
s_pfn_Get_Process_Memory_Info = (GetProcessMemoryInfo_Proc)
GetProcAddress (hm_psapi, "GetProcessMemoryInfo");
@@ -7348,7 +7352,7 @@ init_winsock (int load_now)
= (void *) GetProcAddress (GetModuleHandle ("kernel32.dll"),
"SetHandleInformation");
- winsock_lib = LoadLibrary ("Ws2_32.dll");
+ winsock_lib = LoadLibraryEx ("Ws2_32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (winsock_lib != NULL)
{
@@ -9316,7 +9320,8 @@ w32_delayed_load (Lisp_Object library_id)
wchar_t name_w[MAX_PATH];
filename_to_utf16 (SSDATA (dll), name_w);
- dll_handle = LoadLibraryW (name_w);
+ dll_handle = LoadLibraryExW (name_w, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR|LOAD_LIBRARY_SEARCH_SYSTEM32);
+
if (dll_handle)
{
res = GetModuleFileNameW (dll_handle, name_w,
@@ -9330,7 +9335,8 @@ w32_delayed_load (Lisp_Object library_id)
char name_a[MAX_PATH];
filename_to_ansi (SSDATA (dll), name_a);
- dll_handle = LoadLibraryA (name_a);
+ dll_handle = LoadLibraryExA (name_a, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR|LOAD_LIBRARY_SEARCH_SYSTEM32);
+
if (dll_handle)
{
res = GetModuleFileNameA (dll_handle, name_a,
@@ -9590,7 +9596,7 @@ maybe_load_unicows_dll (void)
{
if (os_subtype == OS_9X)
{
- HANDLE ret = LoadLibrary ("Unicows.dll");
+ HANDLE ret = LoadLibraryEx ("Unicows.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (ret)
{
/* These two functions are present on Windows 9X as stubs
@@ -9638,7 +9644,7 @@ maybe_load_unicows_dll (void)
multiByteToWideCharFlags = 0;
else
multiByteToWideCharFlags = MB_ERR_INVALID_CHARS;
- return LoadLibrary ("Gdi32.dll");
+ return LoadLibraryEx ("Gdi32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
}
}
diff --git a/src/w32fns.c b/src/w32fns.c
index 6b93afa8b8..c566ebc24e 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -10897,9 +10898,9 @@ w32_backtrace (void **buffer, int limit)
static CaptureStackBackTrace_proc s_pfn_CaptureStackBackTrace = NULL;
HMODULE hm_kernel32 = NULL;
if (!s_pfn_CaptureStackBackTrace)
{
- hm_kernel32 = LoadLibrary ("Kernel32.dll");
+ hm_kernel32 = LoadLibraryEx ("Kernel32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
s_pfn_CaptureStackBackTrace =
(CaptureStackBackTrace_proc) GetProcAddress (hm_kernel32,
"RtlCaptureStackBackTrace");
diff --git a/src/w32heap.c b/src/w32heap.c
index cd1324cc86..30b4304aaa 100644
--- a/src/w32heap.c
+++ b/src/w32heap.c
@@ -249,7 +249,7 @@ init_heap (void)
#ifndef MINGW_W64
/* Set the low-fragmentation heap for OS before Vista. */
- HMODULE hm_kernel32dll = LoadLibrary ("kernel32.dll");
+ HMODULE hm_kernel32dll = LoadLibraryEx ("kernel32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
HeapSetInformation_Proc s_pfn_Heap_Set_Information = (HeapSetInformation_Proc) GetProcAddress (hm_kernel32dll, "HeapSetInformation");
if (s_pfn_Heap_Set_Information != NULL)
{
@@ -279,7 +279,7 @@ init_heap (void)
/* Find the RtlCreateHeap function. Headers for this function
are provided with the w32 DDK, but the function is available
in ntdll.dll since XP. */
- HMODULE hm_ntdll = LoadLibrary ("ntdll.dll");
+ HMODULE hm_ntdll = LoadLibraryEx ("ntdll.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
RtlCreateHeap_Proc s_pfn_Rtl_Create_Heap
= (RtlCreateHeap_Proc) GetProcAddress (hm_ntdll, "RtlCreateHeap");
/* Specific parameters for the private heap. */
diff --git a/src/w32proc.c b/src/w32proc.c
index 4459ebe324..7b51043c99 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -3609,7 +3620,7 @@ w32_compare_strings (const char *s1, const char *s2, char *locname,
if (os_subtype == OS_9X)
{
pCompareStringW =
- (CompareStringW_Proc) GetProcAddress (LoadLibrary ("Unicows.dll"),
+ (CompareStringW_Proc) GetProcAddress (LoadLibraryEx ("Unicows.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32),
"CompareStringW");
if (!pCompareStringW)
{
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 7:58 Emacs master, security concernes, ms-windows Fabrice Popineau
@ 2017-09-14 13:33 ` Óscar Fuentes
2017-09-14 13:46 ` Fabrice Popineau
2017-09-14 17:18 ` Eli Zaretskii
2017-09-14 14:13 ` Andy Moreton
2017-09-14 17:08 ` Eli Zaretskii
2 siblings, 2 replies; 10+ messages in thread
From: Óscar Fuentes @ 2017-09-14 13:33 UTC (permalink / raw
To: emacs-devel
Fabrice Popineau <fabrice.popineau@gmail.com> writes:
> Since there seems to be a lot of concerns wrt to security,
> I am submitting the attached patch.
>
> The reason for this patch is to limit the search for dlls loaded at
> runtime to the win32 system directory and/or the emacs application
> directory.
> In the current state, dlls can be picked up in any directory in the path.
> Some one could fake one of these dlls (xpm, png, etc.) and use it for
> mean reasons.
> It is not bullet proof, but it levels up security and
> many other projects have applied such a restriction.
>
> Best regards,
>
> Fabrice
> diff --git a/src/unexw32.c b/src/unexw32.c
> index 5259b2a52b..10f720f734 100644
> --- a/src/unexw32.c
> +++ b/src/unexw32.c
> @@ -772,7 +820,7 @@ unexec (const char *new_name, const char *old_name)
> {
> PIMAGE_DOS_HEADER dos_header;
> PIMAGE_NT_HEADERS nt_header;
> - HANDLE hImagehelp = LoadLibrary ("imagehlp.dll");
> + HANDLE hImagehelp = LoadLibraryEx ("imagehlp.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
According do MSDN, LoadLibraryEx does not support
LOAD_LIBRARY_SEARCH_SYSTEM32 nor LOAD_LIBRARY_SEARCH_APPLICATION_DIR on
Windows XP, and on Windows 7 and some other version it requires certain
security system patch to be installed.
Apart from that, the security provided by this approach is questionable.
If the attacker has enough control to install a DLL and modify the PATH,
it is game over.
Finally, this patch can be a hindrance for those who build Emacs. After
the build is over, you need to copy the required extra dlls (for image
support, etc) to the build binary directory to test or use Emacs. Not a
huge inconvenience, but it isn't irrelevant either.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 13:33 ` Óscar Fuentes
@ 2017-09-14 13:46 ` Fabrice Popineau
2017-09-14 15:20 ` Óscar Fuentes
2017-09-14 17:18 ` Eli Zaretskii
1 sibling, 1 reply; 10+ messages in thread
From: Fabrice Popineau @ 2017-09-14 13:46 UTC (permalink / raw
To: Óscar Fuentes; +Cc: Emacs developers
[-- Attachment #1: Type: text/plain, Size: 985 bytes --]
2017-09-14 15:33 GMT+02:00 Óscar Fuentes <ofv@wanadoo.es>:
> Fabrice Popineau <fabrice.popineau@gmail.com> writes:
>
> Apart from that, the security provided by this approach is questionable.
> If the attacker has enough control to install a DLL and modify the PATH,
> it is game over.
>
At the moment, any libpng.dll (for example) on the PATH can be loaded by
emacs.
With this restriction, only the one provided with an emacs package will be.
I came to 'fix' this because I am using the Anaconda Python distribution
which also
provides its own set of dlls. At some point I got a failure because their
dlls got loaded,
instead of the mingw64 ones.
Finally, this patch can be a hindrance for those who build Emacs. After
> the build is over, you need to copy the required extra dlls (for image
> support, etc) to the build binary directory to test or use Emacs. Not a
> huge inconvenience, but it isn't irrelevant either.
>
>
That's true.
--
Fabrice
[-- Attachment #2: Type: text/html, Size: 1792 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 7:58 Emacs master, security concernes, ms-windows Fabrice Popineau
2017-09-14 13:33 ` Óscar Fuentes
@ 2017-09-14 14:13 ` Andy Moreton
2017-09-14 17:08 ` Eli Zaretskii
2 siblings, 0 replies; 10+ messages in thread
From: Andy Moreton @ 2017-09-14 14:13 UTC (permalink / raw
To: emacs-devel
On Thu 14 Sep 2017, Fabrice Popineau wrote:
> Since there seems to be a lot of concerns wrt to security,
> I am submitting the attached patch.
>
> The reason for this patch is to limit the search for dlls loaded at
> runtime to the win32 system directory and/or the emacs application
> directory.
> In the current state, dlls can be picked up in any directory in the path.
> Some one could fake one of these dlls (xpm, png, etc.) and use it for
> mean reasons.
> It is not bullet proof, but it levels up security and
> many other projects have applied such a restriction.
Restricting the path used to locate DLLs is reasonable, but this is too
strict.
For an emacs built from source and run from the build tree (i.e. not
installed), this requires copying all of the distro DLLs to the emacs
build directory, and keeping them up to date.
Also note that the patch uses LOAD_LIBRARY_SEARCH_* flags, which won't
work at all on Windows XP, or on newer Windows versions that do not have
the correct updates installed. See:
Dynamic-Link Library Security:
https://msdn.microsoft.com/en-us/library/windows/desktop/ff919712(v=vs.85).aspx
Dynamic-Link Library Search Order:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682586(v=vs.85).aspx
LoadLibraryEx:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms684179(v=vs.85).aspx
AndyM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 13:46 ` Fabrice Popineau
@ 2017-09-14 15:20 ` Óscar Fuentes
2017-09-14 15:56 ` Fabrice Popineau
2017-09-14 17:24 ` Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: Óscar Fuentes @ 2017-09-14 15:20 UTC (permalink / raw
To: emacs-devel
Fabrice Popineau <fabrice.popineau@centralesupelec.fr> writes:
> At the moment, any libpng.dll (for example) on the PATH can be loaded by
> emacs.
> With this restriction, only the one provided with an emacs package will be.
>
> I came to 'fix' this because I am using the Anaconda Python distribution
> which also
> provides its own set of dlls. At some point I got a failure because their
> dlls got loaded,
> instead of the mingw64 ones.
I suffered a similar problem on the past.
The real concern about this patch is that it raises quite a bit the
minimum supported OS version, which is a big no-no, AFAIK.
Although I'm not sure what happens if you pass an unsupported flag to
LoadLibraryEx.
It is possible to implement your idea by using explicit paths.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 15:20 ` Óscar Fuentes
@ 2017-09-14 15:56 ` Fabrice Popineau
2017-09-14 17:27 ` Eli Zaretskii
2017-09-14 17:24 ` Eli Zaretskii
1 sibling, 1 reply; 10+ messages in thread
From: Fabrice Popineau @ 2017-09-14 15:56 UTC (permalink / raw
To: Óscar Fuentes; +Cc: Emacs developers
[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]
2017-09-14 17:20 GMT+02:00 Óscar Fuentes <ofv@wanadoo.es>:
> Fabrice Popineau <fabrice.popineau@centralesupelec.fr> writes:
>
> > At the moment, any libpng.dll (for example) on the PATH can be loaded by
> > emacs.
> > With this restriction, only the one provided with an emacs package will
> be.
> >
> > I came to 'fix' this because I am using the Anaconda Python distribution
> > which also
> > provides its own set of dlls. At some point I got a failure because their
> > dlls got loaded,
> > instead of the mingw64 ones.
>
> I suffered a similar problem on the past.
>
> The real concern about this patch is that it raises quite a bit the
> minimum supported OS version, which is a big no-no, AFAIK.
>
> Although I'm not sure what happens if you pass an unsupported flag to
> LoadLibraryEx.
>
>
> It is possible to implement your idea by using explicit paths.
>
Yes, emacs could itself decide where to look for these dlls.
At least for the ones that are non system.
And the system ones should be looked for only in the System32 directory.
I know that my patch was a radical one. I mainly wanted to raise the issue.
Fabrice
[-- Attachment #2: Type: text/html, Size: 1939 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 7:58 Emacs master, security concernes, ms-windows Fabrice Popineau
2017-09-14 13:33 ` Óscar Fuentes
2017-09-14 14:13 ` Andy Moreton
@ 2017-09-14 17:08 ` Eli Zaretskii
2 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2017-09-14 17:08 UTC (permalink / raw
To: Fabrice Popineau; +Cc: emacs-devel
> From: Fabrice Popineau <fabrice.popineau@gmail.com>
> Date: Thu, 14 Sep 2017 09:58:36 +0200
>
> Since there seems to be a lot of concerns wrt to security,
> I am submitting the attached patch.
>
> The reason for this patch is to limit the search for dlls loaded at
> runtime to the win32 system directory and/or the emacs application
> directory.
> In the current state, dlls can be picked up in any directory in the path.
> Some one could fake one of these dlls (xpm, png, etc.) and use it for
> mean reasons.
I see your point, and I'm sympathetic to the concerns. However, given
the accepted practices of installing packages on Windows, I don't
think we can do this by default. Many times people install optional
libraries in separate directories and just add them to PATH. Some
even install each individual package into its own directory. I just
had a conversation off-list with one such user. Disabling DLL look up
on PATH by default will completely screw up those users.
In addition, as others pointed out, this mode is not supported before
Vista, so we shouldn't pass that constant to LoadLibraryEx on systems
that don't support it, even if we accept such a feature as an optional
one.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 13:33 ` Óscar Fuentes
2017-09-14 13:46 ` Fabrice Popineau
@ 2017-09-14 17:18 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2017-09-14 17:18 UTC (permalink / raw
To: Óscar Fuentes; +Cc: emacs-devel
> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Thu, 14 Sep 2017 15:33:40 +0200
>
> Finally, this patch can be a hindrance for those who build Emacs. After
> the build is over, you need to copy the required extra dlls (for image
> support, etc) to the build binary directory to test or use Emacs. Not a
> huge inconvenience, but it isn't irrelevant either.
Indeed. And in addition, there are Emacs modules, which are also
DLLs, but loaded from different places. (The patch didn't touch that
facility, but they should probably get the same treatment.)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 15:20 ` Óscar Fuentes
2017-09-14 15:56 ` Fabrice Popineau
@ 2017-09-14 17:24 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2017-09-14 17:24 UTC (permalink / raw
To: Óscar Fuentes; +Cc: emacs-devel
> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Thu, 14 Sep 2017 17:20:39 +0200
>
> The real concern about this patch is that it raises quite a bit the
> minimum supported OS version, which is a big no-no, AFAIK.
That could be solved by having this code run only on systems that
support the flag.
So I think the real concern is the other issues that this patch
raises.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Emacs master, security concernes, ms-windows
2017-09-14 15:56 ` Fabrice Popineau
@ 2017-09-14 17:27 ` Eli Zaretskii
0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2017-09-14 17:27 UTC (permalink / raw
To: Fabrice Popineau; +Cc: ofv, emacs-devel
> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Thu, 14 Sep 2017 17:56:46 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>
>
> And the system ones should be looked for only in the System32 directory.
Actually, I think all of the system DLLs we try to load dynamically
don't need to be loaded except on Windows 9X. So we could install a
patch that refrains from loading dynamically any system DLLs on the NT
family of Windows. Any volunteers?
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-09-14 17:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-14 7:58 Emacs master, security concernes, ms-windows Fabrice Popineau
2017-09-14 13:33 ` Óscar Fuentes
2017-09-14 13:46 ` Fabrice Popineau
2017-09-14 15:20 ` Óscar Fuentes
2017-09-14 15:56 ` Fabrice Popineau
2017-09-14 17:27 ` Eli Zaretskii
2017-09-14 17:24 ` Eli Zaretskii
2017-09-14 17:18 ` Eli Zaretskii
2017-09-14 14:13 ` Andy Moreton
2017-09-14 17:08 ` Eli Zaretskii
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.