Skip to content

Commit

Permalink
Port file extension and character restrictions for vms from jk2mv.
Browse files Browse the repository at this point in the history
- Use GetFullPathNameA, GetFinalPathNameByHandleA and realpath to actually resolve paths when validating filenames
- Use ERR_DROP instead of ERR_FATAL when a VM tries to access a forbidden extension
- Check for more forbidden extensions
- Prevent the VM from accessing files with unwanted characters in their names (just block the access and print a warning, don't drop)
  • Loading branch information
Daggolin committed Nov 2, 2023
1 parent cf4fb13 commit cf10de6
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 4 deletions.
90 changes: 86 additions & 4 deletions codemp/qcommon/files.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,77 @@ FILE* missingFiles = NULL;
# endif
#endif

const char *get_filename_ext(const char *filename) {
const char *dot = strrchr(filename, '.');
if (!dot || dot == filename) return "";
return dot + 1;
}

// We don't want VMs to be able to access the following file extensions
static char *invalidExtensions[] = {
"dll",
"exe",
"bat",
"cmd",
"dylib",
"so",
"qvm",
"pk3",
};
static int invalidExtensionsAmount = sizeof(invalidExtensions) / sizeof(invalidExtensions[0]);

qboolean FS_IsInvalidExtension( const char *ext ) {
int i;

// Compare against the list of forbidden extensions
for ( i = 0; i < invalidExtensionsAmount; i++ ) {
if ( !Q_stricmp(ext, invalidExtensions[i]) )
return qtrue;
}

// Additional check we don't currently need, but if support for another platform is added and the list above is not
// updated we catch it with this check.
if ( !Q_stricmp(va(".%s", ext), DLL_EXT) )
return qtrue;

return qfalse;
}

// Invalid characters. Originally intended to be OS-specific, but considering that VMs run on different systems it's
// probably not a bad idea to share the same behavior on all systems.
qboolean FS_ContainsInvalidCharacters( const char *filename ) {
static char *invalidCharacters = "<>:\"|?*";
char *ptr = invalidCharacters;

while ( *ptr ) {
if ( strchr(filename, *ptr) )
return qtrue;
ptr++;
}

return qfalse;
}

qboolean FS_IsInvalidWriteOSPath(const char *ospath) {
const char *resolved;
const char *realPath;

// Resolve paths
resolved = Sys_ResolvePath( ospath );
if ( !strlen(resolved) ) return qtrue;
realPath = Sys_RealPath( resolved );
if ( !strlen(realPath) ) return qtrue;

// Check all three, the original input, the resolved and the real path, in case there is a symlink
// NOTE: Checking the ospath shouldn't be required, because we resolved the path, but let's check it anyway.
if ( FS_IsInvalidExtension(get_filename_ext(ospath)) || FS_IsInvalidExtension(get_filename_ext(resolved)) || FS_IsInvalidExtension(get_filename_ext(realPath)) ) {
Com_Printf( "FS_IsInvalidWriteOSPath: blocked writing binary file \"%s\" (%s) [%s].\n", ospath, resolved, realPath );
return qtrue;
}

return qfalse;
}

/*
==============
FS_Initialized
Expand Down Expand Up @@ -572,11 +643,10 @@ ERR_FATAL if trying to maniuplate a file with the platform library, or pk3 exten
*/
static void FS_CheckFilenameIsMutable( const char *filename, const char *function )
{
// Check if the filename ends with the library, or pk3 extension
if( COM_CompareExtension( filename, DLL_EXT )
|| COM_CompareExtension( filename, ".pk3" ) )
// Call FS_InvalidWriteOSPath from JK2MV, because it handles symlinks and some Windows specific tricks to bypass these checks.
if ( FS_IsInvalidWriteOSPath(filename) )
{
Com_Error( ERR_FATAL, "%s: Not allowed to manipulate '%s' due "
Com_Error( ERR_DROP, "%s: Not allowed to manipulate '%s' due "
"to %s extension", function, filename, COM_GetExtension( filename ) );
}
}
Expand All @@ -593,6 +663,11 @@ void FS_CopyFile( char *fromOSPath, char *toOSPath ) {
int len;
byte *buf;

if ( FS_ContainsInvalidCharacters(toOSPath) ) {
Com_Printf( "FS_CopyFile: invalid filename (%s)\n", toOSPath );
return;
}

FS_CheckFilenameIsMutable( fromOSPath, __func__ );

Com_Printf( "copy %s to %s\n", fromOSPath, toOSPath );
Expand Down Expand Up @@ -3950,6 +4025,13 @@ int FS_FOpenFileByMode( const char *qpath, fileHandle_t *f, fsMode_t mode ) {

sync = qfalse;

// Only check the unresolved path for invalid characters, the os probably knows what it's doing
if ( FS_ContainsInvalidCharacters(qpath) ) {
Com_Printf( "FS_FOpenFileByMode: invalid filename (%s)\n", qpath );
*f = 0;
return -1;
}

switch( mode ) {
case FS_READ:
r = FS_FOpenFileRead( qpath, f, qtrue );
Expand Down
17 changes: 17 additions & 0 deletions shared/sys/sys_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,7 @@ int main ( int argc, char* argv[] )
{
int i;
char commandLine[ MAX_STRING_CHARS ] = { 0 };
int missingFuncs = Sys_FindFunctions();

Sys_PlatformInit();
CON_Init();
Expand Down Expand Up @@ -770,6 +771,22 @@ int main ( int argc, char* argv[] )

Com_Init (commandLine);

if ( missingFuncs ) {
static const char *missingFuncsError =
"Your system is missing functions this application relies on.\n"
"\n"
"Some features may be unavailable or their behavior may be incorrect.";

// Set the error cvar (the main menu should pick this up and display an error box to the user)
Cvar_Get( "com_errorMessage", missingFuncsError, CVAR_ROM );
Cvar_Set( "com_errorMessage", missingFuncsError );

// Print the error into the console, because we can't always display the main menu (dedicated servers, ...)
Com_Printf( "********************\n" );
Com_Printf( "ERROR: %s\n", missingFuncsError );
Com_Printf( "********************\n" );
}

#ifndef DEDICATED
SDL_version compiled;
SDL_version linked;
Expand Down
4 changes: 4 additions & 0 deletions shared/sys/sys_public.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ void Sys_SetProcessorAffinity( void );

int Sys_PID( void );

const char *Sys_ResolvePath( const char *path );
const char *Sys_RealPath( const char *path );
int Sys_FindFunctions( void );

typedef enum graphicsApi_e
{
GRAPHICS_API_GENERIC,
Expand Down
40 changes: 40 additions & 0 deletions shared/sys/sys_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -613,3 +613,43 @@ void Sys_AnsiColorPrint( const char *msg )
fputs( buffer, stderr );
}
}

/*
===============
Sys_ResolvePath
===============
*/
const char *Sys_ResolvePath( const char *path )
{ // There seems to be no function to resolve paths of files that don't exist
// on unix, so we just return the input path. This shouldn't be an issue,
// as we just need to resolve paths for those on windows anyway.

return path;
}

/*
===============
Sys_RealPath
===============
*/
const char *Sys_RealPath( const char *path )
{
static char realPath[PATH_MAX+1];
if ( realpath(path, realPath) )
return realPath;
return path;
}

/*
===============
Sys_FindFunctions
===============
*/
int Sys_FindFunctions( void )
{
// We only use this function on Windows to find functions that might not be
// available on all OS versions, but that may be required for some Sys_
// functions to be fully operational. On unix we can just return 0.

return 0;
}
69 changes: 69 additions & 0 deletions shared/sys/sys_win32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -591,3 +591,72 @@ void Sys_Sleep( int msec )
Sleep( msec );
#endif
}

/*
===============
Sys_ResolvePath
===============
*/
const char *Sys_ResolvePath( const char *path )
{
static char resolvedPath[MAX_PATH];

if ( !GetFullPathNameA((LPCSTR)path, sizeof(resolvedPath), (LPSTR)resolvedPath, NULL) )
return "";

return resolvedPath;
}

/*
===============
Sys_RealPath
===============
*/

typedef DWORD (WINAPI *_GetFinalPathNameByHandleA)( _In_ HANDLE, _Out_writes_(cchFilePath) LPSTR, _In_ DWORD, _In_ DWORD );
_GetFinalPathNameByHandleA Win_GetFinalPathNameByHandleA;
const char *Sys_RealPath( const char *path )
{
static char realPath[MAX_PATH];
HANDLE fileHandle;
DWORD len;

// Return the original path if the system doesn't support GetFinalPathNameByHandleA
if ( !Win_GetFinalPathNameByHandleA ) return path;

// Get a handle to later resolve symlinks
fileHandle = CreateFileA( (LPCSTR)path, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL );

// If we can't access it return the original instead
if( fileHandle == INVALID_HANDLE_VALUE )
return path;

// Get the resolvedName from the handle
len = Win_GetFinalPathNameByHandleA( fileHandle, (LPSTR)realPath, sizeof(realPath), VOLUME_NAME_NT );

// Close the handle
CloseHandle( fileHandle );

// If it's longer than we can store return "" to disable access
if( len >= sizeof(realPath) )
return "";

return realPath;
}

/*
===============
Sys_FindFunctions
===============
*/
int Sys_FindFunctions( void )
{
int missingFuncs = 0;

// Get the kernel32 handle and try to find the function "GetFinalPathNameByHandleA"
HINSTANCE handle = GetModuleHandleA( "KERNEL32" );
if ( handle ) Win_GetFinalPathNameByHandleA = (_GetFinalPathNameByHandleA)GetProcAddress( handle, "GetFinalPathNameByHandleA" );
if ( !Win_GetFinalPathNameByHandleA ) missingFuncs++;

return missingFuncs;
}

0 comments on commit cf10de6

Please sign in to comment.