From b16fa8eddcf2d9747617f537442e08b075ef4014 Mon Sep 17 00:00:00 2001 From: Alibek Omarov Date: Mon, 24 Apr 2023 02:18:40 +0300 Subject: [PATCH] public: remove Q_strcat and Q_strcpy, and patch the code that uses it --- engine/client/cl_debug.c | 2 +- engine/client/cl_game.c | 2 +- engine/client/cl_main.c | 10 +++++----- engine/client/in_touch.c | 4 ++-- engine/client/ref_common.c | 2 +- engine/client/s_vox.c | 2 +- engine/client/titles.c | 14 ++++++++++---- engine/common/cmd.c | 8 ++++---- engine/common/common.c | 9 +++++---- engine/common/con_utils.c | 6 +++--- engine/common/cvar.c | 4 ++-- engine/common/infostring.c | 4 +++- engine/common/mod_bmodel.c | 2 +- engine/common/net_chan.c | 4 ++-- engine/common/net_encode.c | 2 +- engine/common/sequence.c | 2 +- engine/platform/posix/sys_posix.c | 11 ++++++----- engine/server/sv_client.c | 8 ++++---- filesystem/VFileSystem009.cpp | 17 ++++++++++------- filesystem/filesystem.c | 2 +- public/crtlib.c | 5 ++++- public/crtlib.h | 2 -- public/tests/test_strings.c | 3 --- 23 files changed, 68 insertions(+), 57 deletions(-) diff --git a/engine/client/cl_debug.c b/engine/client/cl_debug.c index 1467ebd2..ec686ffe 100644 --- a/engine/client/cl_debug.c +++ b/engine/client/cl_debug.c @@ -45,7 +45,7 @@ const char *CL_MsgInfo( int cmd ) { static string sz; - Q_strcpy( sz, "???" ); + Q_strncpy( sz, "???", sizeof( sz )); if( cmd >= 0 && cmd <= svc_lastmsg ) { diff --git a/engine/client/cl_game.c b/engine/client/cl_game.c index 8e57816f..eae4d320 100644 --- a/engine/client/cl_game.c +++ b/engine/client/cl_game.c @@ -2569,7 +2569,7 @@ const char *pfnGetGameDirectory( void ) { static char szGetGameDir[MAX_SYSPATH]; - Q_strcpy( szGetGameDir, GI->gamefolder ); + Q_strncpy( szGetGameDir, GI->gamefolder, sizeof( szGetGameDir )); return szGetGameDir; } diff --git a/engine/client/cl_main.c b/engine/client/cl_main.c index 350ae7dc..9a8111c1 100644 --- a/engine/client/cl_main.c +++ b/engine/client/cl_main.c @@ -1294,15 +1294,15 @@ void CL_Rcon_f( void ) NET_Config( true, false ); // allow remote - Q_strcat( message, "rcon " ); - Q_strcat( message, rcon_password.string ); - Q_strcat( message, " " ); + Q_strncat( message, "rcon ", sizeof( message )); + Q_strncat( message, rcon_password.string, sizeof( message )); + Q_strncat( message, " ", sizeof( message ) ); for( i = 1; i < Cmd_Argc(); i++ ) { Cmd_Escape( command, Cmd_Argv( i ), sizeof( command )); - Q_strcat( message, command ); - Q_strcat( message, " " ); + Q_strncat( message, command, sizeof( message )); + Q_strncat( message, " ", sizeof( message )); } if( cls.state >= ca_connected ) diff --git a/engine/client/in_touch.c b/engine/client/in_touch.c index f4d38f15..198c443b 100644 --- a/engine/client/in_touch.c +++ b/engine/client/in_touch.c @@ -1498,9 +1498,9 @@ static void Touch_EditMove( touchEventType type, int fingerID, float x, float y, touch.hidebutton->flags &= ~TOUCH_FL_HIDE; if( FBitSet( button->flags, TOUCH_FL_HIDE )) - Q_strcpy( touch.hidebutton->texturefile, "touch_default/edit_show" ); + Q_strncpy( touch.hidebutton->texturefile, "touch_default/edit_show", sizeof( touch.hidebutton->texturefile )); else - Q_strcpy( touch.hidebutton->texturefile, "touch_default/edit_hide" ); + Q_strncpy( touch.hidebutton->texturefile, "touch_default/edit_hide", sizeof( touch.hidebutton->texturefile )); } } if( type == event_motion ) // shutdown button move diff --git a/engine/client/ref_common.c b/engine/client/ref_common.c index 81e6dd1d..106688a9 100644 --- a/engine/client/ref_common.c +++ b/engine/client/ref_common.c @@ -512,7 +512,7 @@ static void R_GetRendererName( char *dest, size_t size, const char *opt ) else { // full path - Q_strcpy( dest, opt ); + Q_strncpy( dest, opt, size ); } } diff --git a/engine/client/s_vox.c b/engine/client/s_vox.c index 73818943..d96a4b3a 100644 --- a/engine/client/s_vox.c +++ b/engine/client/s_vox.c @@ -142,7 +142,7 @@ static const char *VOX_GetDirectory( char *szpath, const char *psz, int nsize ) if( !p ) { - Q_strcpy( szpath, "vox/" ); + Q_strncpy( szpath, "vox/", nsize ); return psz; } diff --git a/engine/client/titles.c b/engine/client/titles.c index 4442ef76..bf5e283d 100644 --- a/engine/client/titles.c +++ b/engine/client/titles.c @@ -218,6 +218,7 @@ void CL_TextMessageParse( byte *pMemFile, int fileSize ) client_textmessage_t textMessages[MAX_MESSAGES]; int i, nameHeapSize, textHeapSize, messageSize, nameOffset; int messageCount, lastNamePos; + size_t textHeapSizeRemaining; lastNamePos = 0; lineNumber = 0; @@ -252,7 +253,7 @@ void CL_TextMessageParse( byte *pMemFile, int fileSize ) Con_Reportf( "TextMessage: unexpected '}' found, line %d\n", lineNumber ); return; } - Q_strcpy( currentName, trim ); + Q_strncpy( currentName, trim, sizeof( currentName )); break; case MSGFILE_TEXT: if( IsEndOfText( trim )) @@ -266,7 +267,7 @@ void CL_TextMessageParse( byte *pMemFile, int fileSize ) return; } - Q_strcpy( nameHeap + lastNamePos, currentName ); + Q_strncpy( nameHeap + lastNamePos, currentName, sizeof( nameHeap ) - lastNamePos ); // terminate text in-place in the memory file // (it's temporary memory that will be deleted) @@ -329,15 +330,20 @@ void CL_TextMessageParse( byte *pMemFile, int fileSize ) // copy text & fixup pointers + textHeapSizeRemaining = textHeapSize; pCurrentText = pNameHeap + nameHeapSize; for( i = 0; i < messageCount; i++ ) { + size_t currentTextSize = Q_strlen( clgame.titles[i].pMessage ) + 1; + clgame.titles[i].pName = pNameHeap; // adjust name pointer (parallel buffer) - Q_strcpy( pCurrentText, clgame.titles[i].pMessage ); // copy text over + Q_strncpy( pCurrentText, clgame.titles[i].pMessage, textHeapSizeRemaining ); // copy text over clgame.titles[i].pMessage = pCurrentText; + pNameHeap += Q_strlen( pNameHeap ) + 1; - pCurrentText += Q_strlen( pCurrentText ) + 1; + pCurrentText += currentTextSize; + textHeapSizeRemaining -= currentTextSize; } if(( pCurrentText - (char *)clgame.titles ) != ( textHeapSize + nameHeapSize + messageSize )) diff --git a/engine/common/cmd.c b/engine/common/cmd.c index 17ae6b03..e8d514c7 100644 --- a/engine/common/cmd.c +++ b/engine/common/cmd.c @@ -1162,13 +1162,13 @@ void Cmd_ForwardToServer( void ) str[0] = 0; if( Q_stricmp( Cmd_Argv( 0 ), "cmd" )) { - Q_strcat( str, Cmd_Argv( 0 )); - Q_strcat( str, " " ); + Q_strncat( str, Cmd_Argv( 0 ), sizeof( str )); + Q_strncat( str, " ", sizeof( str )); } if( Cmd_Argc() > 1 ) - Q_strcat( str, Cmd_Args( )); - else Q_strcat( str, "\n" ); + Q_strncat( str, Cmd_Args( ), sizeof( str )); + else Q_strncat( str, "\n", sizeof( str )); MSG_WriteString( &cls.netchan.message, str ); } diff --git a/engine/common/common.c b/engine/common/common.c index 3e1fcaae..446a25c5 100644 --- a/engine/common/common.c +++ b/engine/common/common.c @@ -1006,7 +1006,7 @@ pfnGetGameDir void GAME_EXPORT pfnGetGameDir( char *szGetGameDir ) { if( !szGetGameDir ) return; - Q_strcpy( szGetGameDir, GI->gamefolder ); + Q_strncpy( szGetGameDir, GI->gamefolder, sizeof( GI->gamefolder )); } qboolean COM_IsSafeFileToDownload( const char *filename ) @@ -1070,13 +1070,15 @@ const char *COM_GetResourceTypeName( resourcetype_t restype ) char *_copystring( poolhandle_t mempool, const char *s, const char *filename, int fileline ) { + size_t size; char *b; if( !s ) return NULL; if( !mempool ) mempool = host.mempool; - b = _Mem_Alloc( mempool, Q_strlen( s ) + 1, false, filename, fileline ); - Q_strcpy( b, s ); + size = Q_strlen( s ) + 1; + b = _Mem_Alloc( mempool, size, false, filename, fileline ); + Q_strncpy( b, s, size ); return b; } @@ -1101,7 +1103,6 @@ void *GAME_EXPORT pfnSequenceGet( const char *fileName, const char *entryName ) { Msg( "Sequence_Get: file %s, entry %s\n", fileName, entryName ); - return Sequence_Get( fileName, entryName ); } diff --git a/engine/common/con_utils.c b/engine/common/con_utils.c index 9c11cd7f..c41206b6 100644 --- a/engine/common/con_utils.c +++ b/engine/common/con_utils.c @@ -571,7 +571,7 @@ qboolean Cmd_GetKeysList( const char *s, char *completedname, int length ) const char *keyname = Key_KeynumToString( i ); if(( *s == '*' ) || !Q_strnicmp( keyname, s, len)) - Q_strcpy( keys[numkeys++], keyname ); + Q_strncpy( keys[numkeys++], keyname, sizeof( keys[0] )); } if( !numkeys ) return false; @@ -765,7 +765,7 @@ qboolean Cmd_GetGamesList( const char *s, char *completedname, int length ) for( i = 0, numgamedirs = 0; i < FI->numgames; i++ ) { if(( *s == '*' ) || !Q_strnicmp( FI->games[i]->gamefolder, s, len)) - Q_strcpy( gamedirs[numgamedirs++], FI->games[i]->gamefolder ); + Q_strncpy( gamedirs[numgamedirs++], FI->games[i]->gamefolder, sizeof( gamedirs[0] )); } if( !numgamedirs ) return false; @@ -826,7 +826,7 @@ qboolean Cmd_GetCDList( const char *s, char *completedname, int length ) for( i = 0, numcdcommands = 0; i < 8; i++ ) { if(( *s == '*' ) || !Q_strnicmp( cd_command[i], s, len)) - Q_strcpy( cdcommands[numcdcommands++], cd_command[i] ); + Q_strncpy( cdcommands[numcdcommands++], cd_command[i], sizeof( cdcommands[0] )); } if( !numcdcommands ) return false; diff --git a/engine/common/cvar.c b/engine/common/cvar.c index 1026016c..657a1dc7 100644 --- a/engine/common/cvar.c +++ b/engine/common/cvar.c @@ -1119,8 +1119,8 @@ void Cvar_Set_f( void ) len = Q_strlen( Cmd_Argv(i) + 1 ); if( l + len >= MAX_CMD_TOKENS - 2 ) break; - Q_strcat( combined, Cmd_Argv( i )); - if( i != c-1 ) Q_strcat( combined, " " ); + Q_strncat( combined, Cmd_Argv( i ), sizeof( combined )); + if( i != c-1 ) Q_strncat( combined, " ", sizeof( combined )); l += len; } diff --git a/engine/common/infostring.c b/engine/common/infostring.c index c7a4ed75..fc9d56be 100644 --- a/engine/common/infostring.c +++ b/engine/common/infostring.c @@ -277,7 +277,9 @@ qboolean GAME_EXPORT Info_RemoveKey( char *s, const char *key ) if( !Q_strncmp( key, pkey, cmpsize )) { - Q_strcpy( start, s ); // remove this part + size_t size = Q_strlen( s ) + 1; + + memmove( start, s, size ); // remove this part return true; } diff --git a/engine/common/mod_bmodel.c b/engine/common/mod_bmodel.c index e2608486..cb9c087c 100644 --- a/engine/common/mod_bmodel.c +++ b/engine/common/mod_bmodel.c @@ -1827,7 +1827,7 @@ static void Mod_LoadEntities( dbspmodel_t *bmod ) wadstring[MAX_TOKEN - 2] = 0; if( !Q_strchr( wadstring, ';' )) - Q_strcat( wadstring, ";" ); + Q_strncat( wadstring, ";", sizeof( wadstring )); // parse wad pathes for( pszWadFile = strtok( wadstring, ";" ); pszWadFile != NULL; pszWadFile = strtok( NULL, ";" )) diff --git a/engine/common/net_chan.c b/engine/common/net_chan.c index 2f093f3d..9e4a06a2 100644 --- a/engine/common/net_chan.c +++ b/engine/common/net_chan.c @@ -273,8 +273,8 @@ void Netchan_ReportFlow( netchan_t *chan ) Assert( chan != NULL ); - Q_strcpy( incoming, Q_pretifymem((float)chan->flow[FLOW_INCOMING].totalbytes, 3 )); - Q_strcpy( outgoing, Q_pretifymem((float)chan->flow[FLOW_OUTGOING].totalbytes, 3 )); + Q_strncpy( incoming, Q_pretifymem((float)chan->flow[FLOW_INCOMING].totalbytes, 3 ), sizeof( incoming )); + Q_strncpy( outgoing, Q_pretifymem((float)chan->flow[FLOW_OUTGOING].totalbytes, 3 ), sizeof( outgoing )); Con_DPrintf( "Signon network traffic: %s from server, %s to server\n", incoming, outgoing ); } diff --git a/engine/common/net_encode.c b/engine/common/net_encode.c index 8c9c54f2..6dd52019 100644 --- a/engine/common/net_encode.c +++ b/engine/common/net_encode.c @@ -793,7 +793,7 @@ static void Delta_InitFields( void ) pfile = COM_ParseFile( pfile, encodeDll, sizeof( encodeDll )); if( !Q_stricmp( encodeDll, "none" )) - Q_strcpy( encodeFunc, "null" ); + Q_strncpy( encodeFunc, "null", sizeof( encodeFunc )); else pfile = COM_ParseFile( pfile, encodeFunc, sizeof( encodeFunc )); // jump to '{' diff --git a/engine/common/sequence.c b/engine/common/sequence.c index fd78c2fc..187ced7b 100644 --- a/engine/common/sequence.c +++ b/engine/common/sequence.c @@ -1556,7 +1556,7 @@ static void Sequence_ParseFile( const char *fileName, qboolean isGlobal ) byte *buffer; fs_offset_t bufSize = 0; - Q_strcpy( g_sequenceParseFileName, fileName ); + Q_strncpy( g_sequenceParseFileName, fileName, sizeof( g_sequenceParseFileName )); g_sequenceParseFileIsGlobal = isGlobal; buffer = FS_LoadFile( va("sequences/%s.seq", fileName ), &bufSize, true ); diff --git a/engine/platform/posix/sys_posix.c b/engine/platform/posix/sys_posix.c index 85f5500f..364263e6 100644 --- a/engine/platform/posix/sys_posix.c +++ b/engine/platform/posix/sys_posix.c @@ -50,11 +50,12 @@ static qboolean Sys_FindExecutable( const char *baseName, char *buf, size_t size needTrailingSlash = ( envPath[length - 1] == '/' ) ? 0 : 1; if( length + baseNameLength + needTrailingSlash < size ) { - Q_strncpy( buf, envPath, length + 1 ); - if( needTrailingSlash ) - Q_strcpy( buf + length, "/" ); - Q_strcpy( buf + length + needTrailingSlash, baseName ); - buf[length + needTrailingSlash + baseNameLength] = '\0'; + string temp; + + Q_strncpy( temp, envPath, length + 1 ); + Q_snprintf( buf, size, "%s%s%s", + temp, needTrailingSlash ? "/" : "", baseName ); + if( access( buf, X_OK ) == 0 ) return true; } diff --git a/engine/server/sv_client.c b/engine/server/sv_client.c index 9a4e0171..95fc2aaa 100644 --- a/engine/server/sv_client.c +++ b/engine/server/sv_client.c @@ -1050,8 +1050,8 @@ void SV_RemoteCommand( netadr_t from, sizebuf_t *msg ) remaining[0] = 0; for( i = 2; i < Cmd_Argc(); i++ ) { - Q_strcat( remaining, Cmd_Argv( i )); - Q_strcat( remaining, " " ); + Q_strncat( remaining, Cmd_Argv( i ), sizeof( remaining )); + Q_strncat( remaining, " ", sizeof( remaining )); } Cmd_ExecuteString( remaining ); } @@ -3384,8 +3384,8 @@ void SV_ParseCvarValue2( sv_client_t *cl, sizebuf_t *msg ) string name, value; int requestID = MSG_ReadLong( msg ); - Q_strcpy( name, MSG_ReadString( msg )); - Q_strcpy( value, MSG_ReadString( msg )); + Q_strncpy( name, MSG_ReadString( msg ), sizeof( name )); + Q_strncpy( value, MSG_ReadString( msg ), sizeof( value )); if( svgame.dllFuncs2.pfnCvarValue2 != NULL ) svgame.dllFuncs2.pfnCvarValue2( cl->edict, requestID, name, value ); diff --git a/filesystem/VFileSystem009.cpp b/filesystem/VFileSystem009.cpp index cd959bf8..250f494d 100644 --- a/filesystem/VFileSystem009.cpp +++ b/filesystem/VFileSystem009.cpp @@ -37,9 +37,12 @@ GNU General Public License for more details. // PLATFORM platform // CONFIG platform/config +// This is a macro because pointers returned by alloca +// shouldn't leave current scope #define FixupPath( var, str ) \ - char *var = static_cast( alloca( Q_strlen(( str )) + 1 )); \ - CopyAndFixSlashes(( var ),( str )) + const size_t var ## _size = Q_strlen(( str )) + 1; \ + char * const var = static_cast( alloca( var ## _size )); \ + CopyAndFixSlashes( var, ( str ), var ## _size ) static inline bool IsIdGamedir( const char *id ) { @@ -72,9 +75,9 @@ static inline const char *IdToDir( char *dir, size_t size, const char *id ) return fs_rootdir; // give at least root directory } -static inline void CopyAndFixSlashes( char *p, const char *in ) +static inline void CopyAndFixSlashes( char *p, const char *in, size_t size ) { - Q_strcpy( p, in ); + Q_strncpy( p, in, size ); COM_FixSlashes( p ); } @@ -419,12 +422,12 @@ public: if( !Q_strnicmp( sp->filename, p, splen )) { - Q_strcpy( out, p + splen + 1 ); + Q_strncpy( out, p + splen + 1, 512 ); return true; } } - Q_strcpy( out, p ); + Q_strncpy( out, p, 512 ); return false; } @@ -469,7 +472,7 @@ public: char dir[MAX_VA_STRING], fullpath[MAX_VA_STRING]; Q_snprintf( fullpath, sizeof( fullpath ), "%s/%s", IdToDir( dir, sizeof( dir ), id ), path ); - CopyAndFixSlashes( fullpath, path ); + CopyAndFixSlashes( fullpath, path, sizeof( fullpath )); return !!FS_AddPak_Fullpath( fullpath, nullptr, FS_CUSTOM_PATH ); } diff --git a/filesystem/filesystem.c b/filesystem/filesystem.c index 7eb3e480..4bb0d46e 100644 --- a/filesystem/filesystem.c +++ b/filesystem/filesystem.c @@ -1150,7 +1150,7 @@ void FS_LoadGameInfo( const char *rootfolder ) // lock uplevel of gamedir for read\write fs_ext_path = false; - if( rootfolder ) Q_strcpy( fs_gamedir, rootfolder ); + if( rootfolder ) Q_strncpy( fs_gamedir, rootfolder, sizeof( fs_gamedir )); Con_Reportf( "FS_LoadGameInfo( %s )\n", fs_gamedir ); // clear any old pathes diff --git a/public/crtlib.c b/public/crtlib.c index 1c0b3e8e..719c07d0 100644 --- a/public/crtlib.c +++ b/public/crtlib.c @@ -803,7 +803,10 @@ void COM_PathSlashFix( char *path ) len = Q_strlen( path ); if( path[len - 1] != '\\' && path[len - 1] != '/' ) - Q_strcpy( &path[len], "/" ); + { + path[len] = '/'; + path[len + 1] = '\0'; + } } /* diff --git a/public/crtlib.h b/public/crtlib.h index 15c1abca..47ce24b9 100644 --- a/public/crtlib.h +++ b/public/crtlib.h @@ -64,9 +64,7 @@ void Q_strnlwr( const char *in, char *out, size_t size_out ); size_t Q_colorstr( const char *string ); char Q_toupper( const char in ); char Q_tolower( const char in ); -#define Q_strcat( dst, src ) Q_strncat( dst, src, 99999 ) size_t Q_strncat( char *dst, const char *src, size_t siz ); -#define Q_strcpy( dst, src ) Q_strncpy( dst, src, 99999 ) size_t Q_strncpy( char *dst, const char *src, size_t siz ); qboolean Q_isdigit( const char *str ); qboolean Q_isspace( const char *str ); diff --git a/public/tests/test_strings.c b/public/tests/test_strings.c index 2f741239..e9f63d17 100644 --- a/public/tests/test_strings.c +++ b/public/tests/test_strings.c @@ -12,9 +12,6 @@ int Test_Strcpycatcmp( void ) if( Q_strcmp( dst, buf )) return 2; - if( Q_strcpy( dst, buf ) != sizeof( buf ) - 1 ) - return 3; - if( Q_strcmp( dst, buf )) return 4;