Skip to content

Commit

Permalink
loadlibrary: Only load system DLLs from the system directory
Browse files Browse the repository at this point in the history
Inspiration provided by: Daniel Stenberg and Ray Satiro

Bug: https://curl.haxx.se/docs/adv_20160530.html

Ref: Windows DLL hijacking with curl, CVE-2016-4802
  • Loading branch information
captain-caveman2k authored and bagder committed May 30, 2016
1 parent ddf25f6 commit 6df916d
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 8 deletions.
4 changes: 2 additions & 2 deletions lib/Makefile.inc
Expand Up @@ -53,7 +53,7 @@ LIB_CFILES = file.c timeval.c base64.c hostip.c progress.c formdata.c \
http_proxy.c non-ascii.c asyn-ares.c asyn-thread.c curl_gssapi.c \
http_ntlm.c curl_ntlm_wb.c curl_ntlm_core.c curl_sasl.c \
curl_multibyte.c hostcheck.c conncache.c pipeline.c dotdot.c \
x509asn1.c http2.c smb.c curl_endian.c curl_des.c
x509asn1.c http2.c smb.c curl_endian.c curl_des.c system_win32.c

LIB_HFILES = arpa_telnet.h netrc.h file.h timeval.h hostip.h progress.h \
formdata.h cookie.h http.h sendf.h ftp.h url.h dict.h if2ip.h \
Expand All @@ -72,7 +72,7 @@ LIB_HFILES = arpa_telnet.h netrc.h file.h timeval.h hostip.h progress.h \
curl_sasl.h curl_multibyte.h hostcheck.h conncache.h \
curl_setup_once.h multihandle.h setup-vms.h pipeline.h dotdot.h \
x509asn1.h http2.h sigpipe.h smb.h curl_endian.h curl_des.h \
curl_printf.h
curl_printf.h system_win32.h

LIB_RCFILES = libcurl.rc

Expand Down
3 changes: 2 additions & 1 deletion lib/Makefile.vc6
Expand Up @@ -5,7 +5,7 @@
# | (__| |_| | _ <| |___
# \___|\___/|_| \_\_____|
#
# Copyright (C) 1999 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
# Copyright (C) 1999 - 2016, Daniel Stenberg, <daniel@haxx.se>, et al.
#
# This software is licensed as described in the file COPYING, which
# you should have received as part of this distribution. The terms
Expand Down Expand Up @@ -616,6 +616,7 @@ X_OBJS= \
$(DIROBJ)\speedcheck.obj \
$(DIROBJ)\splay.obj \
$(DIROBJ)\ssh.obj \
$(DIROBJ)\system_win32.obj \
$(DIROBJ)\vauth.obj \
$(DIROBJ)\cleartext.obj \
$(DIROBJ)\cram.obj \
Expand Down
7 changes: 4 additions & 3 deletions lib/curl_sspi.c
Expand Up @@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 1998 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 1998 - 2016, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
Expand All @@ -27,6 +27,7 @@
#include <curl/curl.h>
#include "curl_sspi.h"
#include "curl_multibyte.h"
#include "system_win32.h"
#include "warnless.h"

/* The last #include files should be: */
Expand Down Expand Up @@ -117,9 +118,9 @@ CURLcode Curl_sspi_global_init(void)

/* Load SSPI dll into the address space of the calling process */
if(securityDll)
s_hSecDll = LoadLibrary(TEXT("security.dll"));
s_hSecDll = Curl_load_library(TEXT("security.dll"));
else
s_hSecDll = LoadLibrary(TEXT("secur32.dll"));
s_hSecDll = Curl_load_library(TEXT("secur32.dll"));
if(!s_hSecDll)
return CURLE_FAILED_INIT;

Expand Down
130 changes: 130 additions & 0 deletions lib/system_win32.c
@@ -0,0 +1,130 @@
/***************************************************************************
* _ _ ____ _
* Project ___| | | | _ \| |
* / __| | | | |_) | |
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 2016, Steve Holme, <steve_holme@hotmail.com>.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
* are also available at https://curl.haxx.se/docs/copyright.html.
*
* You may opt to use, copy, modify, merge, publish, distribute and/or sell
* copies of the Software, and permit persons to whom the Software is
* furnished to do so, under the terms of the COPYING file.
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
* KIND, either express or implied.
*
***************************************************************************/

#include "curl_setup.h"

#if defined(WIN32)

#if defined(USE_WINDOWS_SSPI) || (!defined(CURL_DISABLE_TELNET) && \
defined(USE_WINSOCK))

#include <curl/curl.h>
#include "system_win32.h"

/* The last #include files should be: */
#include "curl_memory.h"
#include "memdebug.h"

#if !defined(LOAD_WITH_ALTERED_SEARCH_PATH)
#define LOAD_WITH_ALTERED_SEARCH_PATH 0x00000008
#endif

#if !defined(LOAD_LIBRARY_SEARCH_SYSTEM32)
#define LOAD_LIBRARY_SEARCH_SYSTEM32 0x00000800
#endif

/* We use our own typedef here since some headers might lack these */
typedef HMODULE (APIENTRY *LOADLIBRARYEX_FN)(LPCTSTR, HANDLE, DWORD);

/* See function definitions in winbase.h */
#ifdef UNICODE
# ifdef _WIN32_WCE
# define LOADLIBARYEX L"LoadLibraryExW"
# else
# define LOADLIBARYEX "LoadLibraryExW"
# endif
#else
# define LOADLIBARYEX "LoadLibraryExA"
#endif

/*
* Curl_load_library()
*
* This is used to dynamically load DLLs using the most secure method available
* for the version of Windows that we are running on.
*
* Parameters:
*
* filename [in] - The filename or full path of the DLL to load. If only the
* filename is passed then the DLL will be loaded from the
* Windows system directory.
*
* Returns the handle of the module on success; otherwise NULL.
*/
HMODULE Curl_load_library(LPCTSTR filename)
{
HMODULE hModule = NULL;
LOADLIBRARYEX_FN pLoadLibraryEx = NULL;

/* Get a handle to kernel32 so we can access it's functions at runtime */
HMODULE hKernel32 = GetModuleHandle(TEXT("kernel32"));
if(!hKernel32)
return NULL;

/* Attempt to find LoadLibraryEx() which is only available on Windows 2000
and above */
pLoadLibraryEx = (LOADLIBRARYEX_FN) GetProcAddress(hKernel32, LOADLIBARYEX);

This comment has been minimized.

Copy link
@gvanem

gvanem Jun 1, 2016

Contributor

Wouldn't it make more sense to set hKernel32 and pLoadLibraryEx only once? I.e. in win32_init() called by curl_global_init(). Thus the whole file should not depend on USE_WINDOWS_SSPI and/or CURL_DISABLE_TELNET. Much cleaner IMHO.

This comment has been minimized.

Copy link
@bagder

bagder Jun 1, 2016

Member

Agreed! And also that sounds almost in line with the thoughts in #845...

/* Detect if there's already a path in the filename and load the library if
there is. Note: Both back slashes and forward slashes have been supported
since the earlier days of DOS at an API level although they are not
supported by command prompt */
if(_tcspbrk(filename, TEXT("\\/")))
hModule = pLoadLibraryEx ?
pLoadLibraryEx(filename, NULL, LOAD_WITH_ALTERED_SEARCH_PATH) :
LoadLibrary(filename);
/* Detect if KB2533623 is installed, as LOAD_LIBARY_SEARCH_SYSTEM32 is only
supported on Windows Vista, Windows Server 2008, Windows 7 and Windows
Server 2008 R2 with this patch or natively on Windows 8 and above */
else if(pLoadLibraryEx && GetProcAddress(hKernel32, "AddDllDirectory")) {
/* Load the DLL from the Windows system directory */
hModule = pLoadLibraryEx(filename, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
}
else {
/* Attempt to get the Windows system path */
UINT systemdirlen = GetSystemDirectory(NULL, 0);
if(systemdirlen) {
/* Allocate space for the full DLL path (Room for the null terminator
is included in systemdirlen) */
size_t filenamelen = _tcslen(filename);
TCHAR *path = malloc(sizeof(TCHAR) * (systemdirlen + 1 + filenamelen));
if(path && GetSystemDirectory(path, systemdirlen)) {
/* Calculate the full DLL path */
_tcscpy(path + _tcslen(path), TEXT("\\"));
_tcscpy(path + _tcslen(path), filename);

/* Load the DLL from the Windows system directory */
hModule = pLoadLibraryEx ?
pLoadLibraryEx(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH) :
LoadLibrary(path);

free(path);
}
}
}

return hModule;
}

#endif /* USE_WINDOWS_SSPI || (!CURL_DISABLE_TELNET && USE_WINSOCK) */

#endif /* WIN32 */
39 changes: 39 additions & 0 deletions lib/system_win32.h
@@ -0,0 +1,39 @@
#ifndef HEADER_CURL_SYSTEM_WIN32_H
#define HEADER_CURL_SYSTEM_WIN32_H
/***************************************************************************
* _ _ ____ _
* Project ___| | | | _ \| |
* / __| | | | |_) | |
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 2016, Steve Holme, <steve_holme@hotmail.com>.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
* are also available at https://curl.haxx.se/docs/copyright.html.
*
* You may opt to use, copy, modify, merge, publish, distribute and/or sell
* copies of the Software, and permit persons to whom the Software is
* furnished to do so, under the terms of the COPYING file.
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
* KIND, either express or implied.
*
***************************************************************************/

#include "curl_setup.h"

#if defined(WIN32)

#if defined(USE_WINDOWS_SSPI) || (!defined(CURL_DISABLE_TELNET) && \
defined(USE_WINSOCK))

/* This is used to dynamically load DLLs */
HMODULE Curl_load_library(LPCTSTR filename);

#endif /* USE_WINDOWS_SSPI || (!CURL_DISABLE_TELNET && USE_WINSOCK) */

#endif /* WIN32 */

#endif /* HEADER_CURL_SYSTEM_WIN32_H */
3 changes: 2 additions & 1 deletion lib/telnet.c
Expand Up @@ -51,6 +51,7 @@
#include "telnet.h"
#include "connect.h"
#include "progress.h"
#include "system_win32.h"

#define TELOPTS
#define TELCMDS
Expand Down Expand Up @@ -1334,7 +1335,7 @@ static CURLcode telnet_do(struct connectdata *conn, bool *done)

/* OK, so we have WinSock 2.0. We need to dynamically */
/* load ws2_32.dll and get the function pointers we need. */
wsock2 = LoadLibrary(TEXT("WS2_32.DLL"));
wsock2 = Curl_load_library(TEXT("WS2_32.DLL"));
if(wsock2 == NULL) {
failf(data, "failed to load WS2_32.DLL (%d)", ERRNO);
return CURLE_FAILED_INIT;
Expand Down
2 changes: 1 addition & 1 deletion packages/Symbian/group/libcurl.mmp
Expand Up @@ -39,7 +39,7 @@ SOURCE \
asyn-ares.c asyn-thread.c curl_gssapi.c http_ntlm.c curl_ntlm_wb.c \
curl_ntlm_core.c curl_sasl.c vtls/schannel.c curl_multibyte.c \
vtls/darwinssl.c conncache.c curl_sasl_sspi.c smb.c curl_endian.c \
curl_des.c \
curl_des.c system_win32.c \
vauth/vauth.c vauth/cleartext.c vauth/cram.c vauth/digest.c \
vauth/digest_sspi.c vauth/krb5_gssapi.c vauth/krb5_sspi.c \
vauth/ntlm.c vauth/ntlm_sspi.c vauth/oauth2.c vauth/spnego_gssapi.c \
Expand Down

0 comments on commit 6df916d

Please sign in to comment.