From a5c0031ec4ac648261553128894e157ee41889cb Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:22:44 +0200 Subject: Correctly process and indicate error from LoadLibraryExA() function Function save_err_str() checks for error by GetLastError() call. Calling EnumProcessModules() may change or reset it. So call save_err_str() immediately after LoadLibraryExA(). --- dlfcn.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/dlfcn.c b/dlfcn.c index 993808d..c97a870 100644 --- a/dlfcn.c +++ b/dlfcn.c @@ -267,24 +267,33 @@ void *dlopen( const char *file, int mode ) hModule = LoadLibraryExA(lpFileName, NULL, LOAD_WITH_ALTERED_SEARCH_PATH ); - if( MyEnumProcessModules( hCurrentProc, NULL, 0, &dwProcModsAfter ) == 0 ) - dwProcModsAfter = 0; - - /* If the object was loaded with RTLD_LOCAL, add it to list of local - * objects, so that its symbols cannot be retrieved even if the handle for - * the original program file is passed. POSIX says that if the same - * file is specified in multiple invocations, and any of them are - * RTLD_GLOBAL, even if any further invocations use RTLD_LOCAL, the - * symbols will remain global. If number of loaded modules was not - * changed after calling LoadLibraryEx(), it means that library was - * already loaded. - */ if( !hModule ) + { save_err_str( lpFileName ); - else if( (mode & RTLD_LOCAL) && dwProcModsBefore != dwProcModsAfter ) - local_add( hModule ); - else if( !(mode & RTLD_LOCAL) && dwProcModsBefore == dwProcModsAfter ) - local_rem( hModule ); + } + else + { + if( MyEnumProcessModules( hCurrentProc, NULL, 0, &dwProcModsAfter ) == 0 ) + dwProcModsAfter = 0; + + /* If the object was loaded with RTLD_LOCAL, add it to list of local + * objects, so that its symbols cannot be retrieved even if the handle for + * the original program file is passed. POSIX says that if the same + * file is specified in multiple invocations, and any of them are + * RTLD_GLOBAL, even if any further invocations use RTLD_LOCAL, the + * symbols will remain global. If number of loaded modules was not + * changed after calling LoadLibraryEx(), it means that library was + * already loaded. + */ + if( (mode & RTLD_LOCAL) && dwProcModsBefore != dwProcModsAfter ) + { + local_add( hModule ); + } + else if( !(mode & RTLD_LOCAL) && dwProcModsBefore == dwProcModsAfter ) + { + local_rem( hModule ); + } + } } /* Return to previous state of the error-mode bit flags. */ -- cgit v1.2.3-55-g6feb From 4fe419cac3a5732844e5dd9fd52600b054bd0b18 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:23:33 +0200 Subject: Correctly process and indicate error in dlsym() function Function save_err_str() checks for error by GetLastError() call. So ensure that last error is always set when error occurs. --- dlfcn.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/dlfcn.c b/dlfcn.c index c97a870..26135e3 100644 --- a/dlfcn.c +++ b/dlfcn.c @@ -362,10 +362,17 @@ void *dlsym( void *handle, const char *name ) size_t sLen; sLen = VirtualQueryEx( hCurrentProc, _ReturnAddress(), &info, sizeof( info ) ); if( sLen != sizeof( info ) ) + { + if( sLen != 0 ) + SetLastError( ERROR_INVALID_PARAMETER ); goto end; + } hCaller = (HMODULE) info.AllocationBase; - if(!hCaller) + if( !hCaller ) + { + SetLastError( ERROR_INVALID_PARAMETER ); goto end; + } } if( handle != RTLD_NEXT ) @@ -423,6 +430,8 @@ void *dlsym( void *handle, const char *name ) end: if( symbol == NULL ) { + if( GetLastError() == 0 ) + SetLastError( ERROR_PROC_NOT_FOUND ); save_err_str( name ); } -- cgit v1.2.3-55-g6feb From 44589dba9c663eaeed5dbf55a02984133b9ab822 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:23:50 +0200 Subject: Correctly process malloc() error in local_add() malloc() may fail, so propagate this error to caller of dlopen(). --- dlfcn.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/dlfcn.c b/dlfcn.c index 26135e3..7fe4701 100644 --- a/dlfcn.c +++ b/dlfcn.c @@ -72,34 +72,36 @@ static local_object *local_search( HMODULE hModule ) return NULL; } -static void local_add( HMODULE hModule ) +static BOOL local_add( HMODULE hModule ) { local_object *pobject; local_object *nobject; if( hModule == NULL ) - return; + return TRUE; pobject = local_search( hModule ); /* Do not add object again if it's already on the list */ if( pobject ) - return; + return TRUE; for( pobject = &first_object; pobject->next; pobject = pobject->next ); nobject = (local_object*) malloc( sizeof( local_object ) ); - /* Should this be enough to fail local_add, and therefore also fail - * dlopen? - */ if( !nobject ) - return; + { + SetLastError( ERROR_NOT_ENOUGH_MEMORY ); + return FALSE; + } pobject->next = nobject; nobject->next = NULL; nobject->previous = pobject; nobject->hModule = hModule; + + return TRUE; } static void local_rem( HMODULE hModule ) @@ -287,7 +289,12 @@ void *dlopen( const char *file, int mode ) */ if( (mode & RTLD_LOCAL) && dwProcModsBefore != dwProcModsAfter ) { - local_add( hModule ); + if( !local_add( hModule ) ) + { + save_err_str( lpFileName ); + FreeLibrary( hModule ); + hModule = NULL; + } } else if( !(mode & RTLD_LOCAL) && dwProcModsBefore == dwProcModsAfter ) { -- cgit v1.2.3-55-g6feb From 207311ceba4f8e2ebab27ead6e07dfecef392383 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:23:58 +0200 Subject: Correctly process and indicate error when file name is too long --- dlfcn.c | 102 ++++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/dlfcn.c b/dlfcn.c index 7fe4701..ec60423 100644 --- a/dlfcn.c +++ b/dlfcn.c @@ -242,63 +242,71 @@ void *dlopen( const char *file, int mode ) HANDLE hCurrentProc; DWORD dwProcModsBefore, dwProcModsAfter; char lpFileName[MAX_PATH]; - size_t i; - - /* MSDN says backslashes *must* be used instead of forward slashes. */ - for( i = 0 ; i < sizeof(lpFileName) - 1 ; i ++ ) - { - if( !file[i] ) - break; - else if( file[i] == '/' ) - lpFileName[i] = '\\'; - else - lpFileName[i] = file[i]; - } - lpFileName[i] = '\0'; - - hCurrentProc = GetCurrentProcess( ); + size_t i, len; - if( MyEnumProcessModules( hCurrentProc, NULL, 0, &dwProcModsBefore ) == 0 ) - dwProcModsBefore = 0; + len = strlen( file ); - /* POSIX says the search path is implementation-defined. - * LOAD_WITH_ALTERED_SEARCH_PATH is used to make it behave more closely - * to UNIX's search paths (start with system folders instead of current - * folder). - */ - hModule = LoadLibraryExA(lpFileName, NULL, - LOAD_WITH_ALTERED_SEARCH_PATH ); - - if( !hModule ) + if( len >= sizeof( lpFileName ) ) { - save_err_str( lpFileName ); + SetLastError( ERROR_FILENAME_EXCED_RANGE ); + save_err_str( file ); + hModule = NULL; } else { - if( MyEnumProcessModules( hCurrentProc, NULL, 0, &dwProcModsAfter ) == 0 ) - dwProcModsAfter = 0; - - /* If the object was loaded with RTLD_LOCAL, add it to list of local - * objects, so that its symbols cannot be retrieved even if the handle for - * the original program file is passed. POSIX says that if the same - * file is specified in multiple invocations, and any of them are - * RTLD_GLOBAL, even if any further invocations use RTLD_LOCAL, the - * symbols will remain global. If number of loaded modules was not - * changed after calling LoadLibraryEx(), it means that library was - * already loaded. + /* MSDN says backslashes *must* be used instead of forward slashes. */ + for( i = 0; i < len; i++ ) + { + if( file[i] == '/' ) + lpFileName[i] = '\\'; + else + lpFileName[i] = file[i]; + } + lpFileName[len] = '\0'; + + hCurrentProc = GetCurrentProcess( ); + + if( MyEnumProcessModules( hCurrentProc, NULL, 0, &dwProcModsBefore ) == 0 ) + dwProcModsBefore = 0; + + /* POSIX says the search path is implementation-defined. + * LOAD_WITH_ALTERED_SEARCH_PATH is used to make it behave more closely + * to UNIX's search paths (start with system folders instead of current + * folder). */ - if( (mode & RTLD_LOCAL) && dwProcModsBefore != dwProcModsAfter ) + hModule = LoadLibraryExA( lpFileName, NULL, LOAD_WITH_ALTERED_SEARCH_PATH ); + + if( !hModule ) { - if( !local_add( hModule ) ) - { - save_err_str( lpFileName ); - FreeLibrary( hModule ); - hModule = NULL; - } + save_err_str( lpFileName ); } - else if( !(mode & RTLD_LOCAL) && dwProcModsBefore == dwProcModsAfter ) + else { - local_rem( hModule ); + if( MyEnumProcessModules( hCurrentProc, NULL, 0, &dwProcModsAfter ) == 0 ) + dwProcModsAfter = 0; + + /* If the object was loaded with RTLD_LOCAL, add it to list of local + * objects, so that its symbols cannot be retrieved even if the handle for + * the original program file is passed. POSIX says that if the same + * file is specified in multiple invocations, and any of them are + * RTLD_GLOBAL, even if any further invocations use RTLD_LOCAL, the + * symbols will remain global. If number of loaded modules was not + * changed after calling LoadLibraryEx(), it means that library was + * already loaded. + */ + if( (mode & RTLD_LOCAL) && dwProcModsBefore != dwProcModsAfter ) + { + if( !local_add( hModule ) ) + { + save_err_str( lpFileName ); + FreeLibrary( hModule ); + hModule = NULL; + } + } + else if( !(mode & RTLD_LOCAL) && dwProcModsBefore == dwProcModsAfter ) + { + local_rem( hModule ); + } } } } -- cgit v1.2.3-55-g6feb From f6f6dd2dcff42b0f7c0cee047f1e9862412a3f02 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:24:05 +0200 Subject: Test that dlerror() returns non-NULL error for failed calls --- test.c | 57 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/test.c b/test.c index 814aca1..d99fd52 100644 --- a/test.c +++ b/test.c @@ -201,11 +201,14 @@ int main() CLOSE_GLOBAL; RETURN_ERROR; } - else { - error = dlerror( ); - printf( "SUCCESS\tCould not get nonexistent symbol from library handle: %s\n", - error ? error : "" ); + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlsym for nonexistent symbol\n" ); + RETURN_ERROR; } + else + printf( "SUCCESS\tCould not get nonexistent symbol from library handle: %s\n", error ); function = dlsym( global, "function" ); if( !function ) @@ -231,11 +234,14 @@ int main() CLOSE_GLOBAL; RETURN_ERROR; } - else { - error = dlerror( ); - printf( "SUCCESS\tCould not get nonexistent symbol from global handle: %s\n", - error ? error : "" ); + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlsym for nonexistent symbol\n" ); + RETURN_ERROR; } + else + printf( "SUCCESS\tCould not get nonexistent symbol from global handle: %s\n", error ); ret = dlclose( library ); if( ret ) @@ -293,11 +299,14 @@ int main() CLOSE_GLOBAL; RETURN_ERROR; } - else { - error = dlerror( ); - printf( "SUCCESS\tCould not get nonexistent symbol from library handle: %s\n", - error ? error : "" ); + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlsym for nonexistent symbol\n" ); + RETURN_ERROR; } + else + printf( "SUCCESS\tCould not get nonexistent symbol from library handle: %s\n", error ); function = dlsym( global, "function" ); if( function ) @@ -321,11 +330,14 @@ int main() CLOSE_GLOBAL; RETURN_ERROR; } - else { - error = dlerror( ); - printf( "SUCCESS\tDid not get nonexistent local symbol from global handle: %s\n", - error ? error : "" ); + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlsym for nonexistent symbol\n" ); + RETURN_ERROR; } + else + printf( "SUCCESS\tDid not get nonexistent local symbol from global handle: %s\n", error ); library = dlopen( "testdll.dll", RTLD_GLOBAL ); if( !library ) @@ -363,10 +375,15 @@ int main() CLOSE_GLOBAL; RETURN_ERROR; } - else { - error = dlerror( ); - printf( "SUCCESS\tCould not get nonexistent symbol from global handle: %s\n", - error ? error : "" ); + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlsym for nonexistent symbol\n" ); + RETURN_ERROR; + } + else + { + printf( "SUCCESS\tCould not get nonexistent symbol from global handle: %s\n", error ); /* Test that the second call to dlerror() returns null as in the specs See https://github.com/dlfcn-win32/dlfcn-win32/issues/34 */ -- cgit v1.2.3-55-g6feb From 27319bfc884c2817c7c5f4fd1046903dc2e419e0 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:24:12 +0200 Subject: Add tests for non-existent file and file with too long name --- test.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test.c b/test.c index d99fd52..f7a20d7 100644 --- a/test.c +++ b/test.c @@ -83,6 +83,8 @@ int main() int (*nonexistentfunction)( void ); int ret; HMODULE library3; + char toolongfile[32767]; + DWORD code; #ifdef _DEBUG _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE); @@ -93,6 +95,54 @@ int main() _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDOUT); #endif + library = dlopen( "nonexistentfile.dll", RTLD_GLOBAL ); + if( library ) + { + printf( "ERROR\tNon-existent file nonexistentfile.dll was opened via dlopen\n" ); + RETURN_ERROR; + } + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlopen for non-existent file\n" ); + RETURN_ERROR; + } + else + printf( "SUCCESS\tCould not open non-existent file nonexistentfile.dll: %s\n", error ); + + memset( toolongfile, 'X', sizeof( toolongfile ) - 5 ); + memcpy( toolongfile + sizeof( toolongfile ) - 5, ".dll", 5 ); + + library = dlopen( toolongfile, RTLD_GLOBAL ); + if( library ) + { + printf( "ERROR\tFile with too long file name was opened via dlopen\n" ); + RETURN_ERROR; + } + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlopen for file with too long file name\n" ); + RETURN_ERROR; + } + else + printf( "SUCCESS\tCould not open file with too long file name: %s\n", error ); + + library3 = LoadLibraryA( toolongfile ); + code = GetLastError( ); + if( library3 ) + { + printf( "ERROR\tFile with too long file name was opened via WINAPI\n" ); + RETURN_ERROR; + } + else if( code != ERROR_FILENAME_EXCED_RANGE ) + { + printf( "ERROR\tFile with too long file name was processed via WINAPI: %lu\n", (unsigned long)code ); + RETURN_ERROR; + } + else + printf( "SUCCESS\tCould not open file with too long file name via WINAPI: %lu\n", (unsigned long)code ); + library2 = dlopen( "testdll2.dll", RTLD_GLOBAL ); if( !library2 ) { -- cgit v1.2.3-55-g6feb From e646c9ab005d6a4480fa3728ebe9c34182bfedc6 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Thu, 23 May 2019 20:24:17 +0200 Subject: Add test for non-library file --- test.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test.c b/test.c index f7a20d7..0070f8c 100644 --- a/test.c +++ b/test.c @@ -85,6 +85,9 @@ int main() HMODULE library3; char toolongfile[32767]; DWORD code; + char nonlibraryfile[MAX_PATH]; + HANDLE tempfile; + DWORD dummy; #ifdef _DEBUG _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE); @@ -95,6 +98,64 @@ int main() _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDOUT); #endif + ret = GetTempPathA( sizeof( nonlibraryfile ) - sizeof( "temp.dll" ), nonlibraryfile ); + if( ret == 0 || ret > sizeof( nonlibraryfile ) - sizeof( "temp.dll" ) ) + { + printf( "ERROR\tGetTempPath failed\n" ); + RETURN_ERROR; + } + + memcpy( nonlibraryfile + ret, "temp.dll", sizeof( "temp.dll" ) ); + + tempfile = CreateFileA( (LPCSTR) nonlibraryfile, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL ); + if( tempfile == INVALID_HANDLE_VALUE ) + { + printf( "ERROR\tCannot create temporary file %s: %lu\n", nonlibraryfile, (unsigned long)GetLastError( ) ); + RETURN_ERROR; + } + + WriteFile( tempfile, "test content", 12, &dummy, NULL ); + + CloseHandle( tempfile ); + + library3 = LoadLibraryA( nonlibraryfile ); + code = GetLastError( ); + if( library3 ) + { + printf( "ERROR\tNon-library file %s was opened via WINAPI\n", nonlibraryfile ); + CloseHandle( library3 ); + DeleteFileA( nonlibraryfile ); + RETURN_ERROR; + } + else if( code != ERROR_BAD_EXE_FORMAT ) + { + printf( "ERROR\tNon-library file %s was processed via WINAPI: %lu\n", nonlibraryfile, (unsigned long)code ); + DeleteFileA( nonlibraryfile ); + RETURN_ERROR; + } + else + printf( "SUCCESS\tCould not open non-library file %s via WINAPI: %lu\n", nonlibraryfile, (unsigned long)code ); + + library = dlopen( nonlibraryfile, RTLD_GLOBAL ); + if( library ) + { + printf( "ERROR\tNon-library file %s was opened via dlopen\n", nonlibraryfile ); + dlclose( library ); + DeleteFileA( nonlibraryfile ); + RETURN_ERROR; + } + error = dlerror( ); + if( !error ) + { + printf( "ERROR\tNo error from dlopen for non-library file\n" ); + DeleteFileA( nonlibraryfile ); + RETURN_ERROR; + } + else + printf( "SUCCESS\tCould not open non-library file %s: %s\n", nonlibraryfile, error ); + + DeleteFileA( nonlibraryfile ); + library = dlopen( "nonexistentfile.dll", RTLD_GLOBAL ); if( library ) { -- cgit v1.2.3-55-g6feb