From 58f78b4f1cf6157dd9f7558eed5cc004864483f7 Mon Sep 17 00:00:00 2001 From: Andrey Akhmichin <15944199+nekonomicon@users.noreply.github.com> Date: Mon, 14 Nov 2022 08:47:47 +0500 Subject: [PATCH] More safe strncpy usage. --- cl_dll/death.cpp | 4 ++-- cl_dll/hud.cpp | 8 ++++---- cl_dll/hud_spectator.cpp | 2 +- cl_dll/menu.cpp | 6 +++--- cl_dll/saytext.cpp | 2 +- cl_dll/scoreboard.cpp | 4 ++-- cl_dll/statusbar.cpp | 5 +++-- cl_dll/vgui_CustomObjects.cpp | 2 +- cl_dll/vgui_SchemeManager.cpp | 8 ++++---- cl_dll/vgui_ScorePanel.cpp | 2 +- cl_dll/vgui_TeamFortressViewport.cpp | 8 ++++---- cl_dll/vgui_TeamFortressViewport.h | 2 +- dlls/client.cpp | 4 ++-- dlls/teamplay_gamerules.cpp | 21 ++++++++++++++------- 14 files changed, 43 insertions(+), 35 deletions(-) diff --git a/cl_dll/death.cpp b/cl_dll/death.cpp index b4a6539c..90024245 100644 --- a/cl_dll/death.cpp +++ b/cl_dll/death.cpp @@ -207,7 +207,7 @@ int CHudDeathNotice::MsgFunc_DeathMsg( const char *pszName, int iSize, void *pbu else { rgDeathNoticeList[i].KillerColor = GetClientColor( killer ); - strncpy( rgDeathNoticeList[i].szKiller, killer_name, MAX_PLAYER_NAME_LENGTH ); + strncpy( rgDeathNoticeList[i].szKiller, killer_name, MAX_PLAYER_NAME_LENGTH - 1 ); rgDeathNoticeList[i].szKiller[MAX_PLAYER_NAME_LENGTH - 1] = 0; } @@ -224,7 +224,7 @@ int CHudDeathNotice::MsgFunc_DeathMsg( const char *pszName, int iSize, void *pbu else { rgDeathNoticeList[i].VictimColor = GetClientColor( victim ); - strncpy( rgDeathNoticeList[i].szVictim, victim_name, MAX_PLAYER_NAME_LENGTH ); + strncpy( rgDeathNoticeList[i].szVictim, victim_name, MAX_PLAYER_NAME_LENGTH - 1 ); rgDeathNoticeList[i].szVictim[MAX_PLAYER_NAME_LENGTH - 1] = 0; } diff --git a/cl_dll/hud.cpp b/cl_dll/hud.cpp index eae37b7a..f16e30b9 100644 --- a/cl_dll/hud.cpp +++ b/cl_dll/hud.cpp @@ -519,8 +519,8 @@ void CHud::VidInit( void ) sprintf( sz, "sprites/%s.spr", p->szSprite ); m_rghSprites[index] = SPR_Load( sz ); m_rgrcRects[index] = p->rc; - strncpy( &m_rgszSpriteNames[index * MAX_SPRITE_NAME_LENGTH], p->szName, MAX_SPRITE_NAME_LENGTH ); - + strncpy( &m_rgszSpriteNames[index * MAX_SPRITE_NAME_LENGTH], p->szName, MAX_SPRITE_NAME_LENGTH - 1 ); + m_rgszSpriteNames[index * MAX_SPRITE_NAME_LENGTH + ( MAX_SPRITE_NAME_LENGTH - 1 )] = '\0'; index++; } @@ -562,8 +562,8 @@ void CHud::VidInit( void ) sprintf( sz, "sprites/%s.spr", p->szSprite ); m_rghSprites[index] = SPR_Load( sz ); m_rgrcRects[index] = p->rc; - strncpy( &m_rgszSpriteNames[index * MAX_SPRITE_NAME_LENGTH], p->szName, MAX_SPRITE_NAME_LENGTH ); - + strncpy( &m_rgszSpriteNames[index * MAX_SPRITE_NAME_LENGTH], p->szName, MAX_SPRITE_NAME_LENGTH - 1 ); + m_rgszSpriteNames[index * MAX_SPRITE_NAME_LENGTH + ( MAX_SPRITE_NAME_LENGTH - 1 )] = '\0'; index++; } diff --git a/cl_dll/hud_spectator.cpp b/cl_dll/hud_spectator.cpp index a6db99f2..7e115f51 100644 --- a/cl_dll/hud_spectator.cpp +++ b/cl_dll/hud_spectator.cpp @@ -537,7 +537,7 @@ void CHudSpectator::DirectorMessage( int iSize, void *pbuf ) msg->holdtime = READ_FLOAT(); // holdtime msg->fxtime = READ_FLOAT(); // fxtime; - strncpy( m_HUDMessageText[m_lastHudMessage], READ_STRING(), 128 ); + strncpy( m_HUDMessageText[m_lastHudMessage], READ_STRING(), 127 ); m_HUDMessageText[m_lastHudMessage][127] = 0; // text msg->pMessage = m_HUDMessageText[m_lastHudMessage]; diff --git a/cl_dll/menu.cpp b/cl_dll/menu.cpp index e9166979..45147e78 100644 --- a/cl_dll/menu.cpp +++ b/cl_dll/menu.cpp @@ -157,7 +157,7 @@ int CHudMenu::MsgFunc_ShowMenu( const char *pszName, int iSize, void *pbuf ) { if( !m_fWaitingForMore ) // this is the start of a new menu { - strncpy( g_szPrelocalisedMenuString, READ_STRING(), MAX_MENU_STRING ); + strncpy( g_szPrelocalisedMenuString, READ_STRING(), MAX_MENU_STRING - 1 ); } else { @@ -169,13 +169,13 @@ int CHudMenu::MsgFunc_ShowMenu( const char *pszName, int iSize, void *pbuf ) if( !NeedMore ) { // we have the whole string, so we can localise it now - strncpy( g_szMenuString, gHUD.m_TextMessage.BufferedLocaliseTextString( g_szPrelocalisedMenuString ), MAX_MENU_STRING ); + strncpy( g_szMenuString, gHUD.m_TextMessage.BufferedLocaliseTextString( g_szPrelocalisedMenuString ), MAX_MENU_STRING - 1 ); g_szMenuString[MAX_MENU_STRING - 1] = '\0'; // Swap in characters if( KB_ConvertString( g_szMenuString, &temp ) ) { - strncpy( g_szMenuString, temp, MAX_MENU_STRING ); + strncpy( g_szMenuString, temp, MAX_MENU_STRING - 1 ); g_szMenuString[MAX_MENU_STRING - 1] = '\0'; free( temp ); } diff --git a/cl_dll/saytext.cpp b/cl_dll/saytext.cpp index d7a7dd98..35d08bec 100644 --- a/cl_dll/saytext.cpp +++ b/cl_dll/saytext.cpp @@ -133,7 +133,7 @@ int CHudSayText::Draw( float flTime ) static char buf[MAX_PLAYER_NAME_LENGTH + 32]; // draw the first x characters in the player color - strncpy( buf, g_szLineBuffer[i], Q_min(g_iNameLengths[i], MAX_PLAYER_NAME_LENGTH + 32 ) ); + strncpy( buf, g_szLineBuffer[i], Q_min(g_iNameLengths[i], MAX_PLAYER_NAME_LENGTH + 31 ) ); buf[Q_min( g_iNameLengths[i], MAX_PLAYER_NAME_LENGTH + 31 )] = 0; DrawSetTextColor( g_pflNameColors[i][0], g_pflNameColors[i][1], g_pflNameColors[i][2] ); int x = DrawConsoleString( LINE_START, y, buf ); diff --git a/cl_dll/scoreboard.cpp b/cl_dll/scoreboard.cpp index 685aaa9f..007585f5 100644 --- a/cl_dll/scoreboard.cpp +++ b/cl_dll/scoreboard.cpp @@ -522,7 +522,7 @@ int CHudScoreboard::MsgFunc_TeamInfo( const char *pszName, int iSize, void *pbuf if( cl > 0 && cl <= MAX_PLAYERS ) { // set the players team - strncpy( g_PlayerExtraInfo[cl].teamname, READ_STRING(), MAX_TEAM_NAME ); + strncpy( g_PlayerExtraInfo[cl].teamname, READ_STRING(), MAX_TEAM_NAME - 1 ); } // rebuild the list of teams @@ -564,7 +564,7 @@ int CHudScoreboard::MsgFunc_TeamInfo( const char *pszName, int iSize, void *pbuf } m_iNumTeams = Q_max( j, m_iNumTeams ); - strncpy( g_TeamInfo[j].name, g_PlayerExtraInfo[i].teamname, MAX_TEAM_NAME ); + strncpy( g_TeamInfo[j].name, g_PlayerExtraInfo[i].teamname, MAX_TEAM_NAME - 1 ); g_TeamInfo[j].players = 0; } diff --git a/cl_dll/statusbar.cpp b/cl_dll/statusbar.cpp index 79f30b7e..41b5008e 100644 --- a/cl_dll/statusbar.cpp +++ b/cl_dll/statusbar.cpp @@ -141,7 +141,8 @@ void CHudStatusBar::ParseStatusString( int line_num ) GetPlayerInfo( indexval, &g_PlayerInfoList[indexval] ); if( g_PlayerInfoList[indexval].name != NULL ) { - strncpy( szRepString, g_PlayerInfoList[indexval].name, MAX_PLAYER_NAME_LENGTH ); + strncpy( szRepString, g_PlayerInfoList[indexval].name, MAX_PLAYER_NAME_LENGTH - 1 ); + szRepString[MAX_PLAYER_NAME_LENGTH - 1] = '\0'; m_pflNameColors[line_num] = GetClientColor( indexval ); } else @@ -231,7 +232,7 @@ int CHudStatusBar::MsgFunc_StatusText( const char *pszName, int iSize, void *pbu if( line < 0 || line >= MAX_STATUSBAR_LINES ) return 1; - strncpy( m_szStatusText[line], READ_STRING(), MAX_STATUSTEXT_LENGTH ); + strncpy( m_szStatusText[line], READ_STRING(), MAX_STATUSTEXT_LENGTH - 1 ); m_szStatusText[line][MAX_STATUSTEXT_LENGTH - 1] = 0; // ensure it's null terminated ( strncpy() won't null terminate if read string too long) if( m_szStatusText[0] == 0 ) diff --git a/cl_dll/vgui_CustomObjects.cpp b/cl_dll/vgui_CustomObjects.cpp index 0bd63652..d14e59bc 100644 --- a/cl_dll/vgui_CustomObjects.cpp +++ b/cl_dll/vgui_CustomObjects.cpp @@ -154,7 +154,7 @@ void CommandButton::RecalculateText( void ) void CommandButton::setText( const char *text ) { - strncpy( m_sMainText, text, MAX_BUTTON_SIZE ); + strncpy( m_sMainText, text, MAX_BUTTON_SIZE - 1 ); m_sMainText[MAX_BUTTON_SIZE - 1] = 0; RecalculateText(); diff --git a/cl_dll/vgui_SchemeManager.cpp b/cl_dll/vgui_SchemeManager.cpp index dddd268c..e50e5b73 100644 --- a/cl_dll/vgui_SchemeManager.cpp +++ b/cl_dll/vgui_SchemeManager.cpp @@ -205,7 +205,7 @@ CSchemeManager::CSchemeManager( int xRes, int yRes ) static const int tokenSize = 64; char paramName[tokenSize], paramValue[tokenSize]; - strncpy( paramName, token, tokenSize ); + strncpy( paramName, token, tokenSize - 1 ); paramName[tokenSize-1] = 0; // ensure null termination // get the '=' character @@ -225,7 +225,7 @@ CSchemeManager::CSchemeManager( int xRes, int yRes ) // get paramValue pFile = gEngfuncs.COM_ParseFile( pFile, token ); - strncpy( paramValue, token, tokenSize ); + strncpy( paramValue, token, tokenSize - 1 ); paramValue[tokenSize-1] = 0; // ensure null termination // is this a new scheme? @@ -278,7 +278,7 @@ CSchemeManager::CSchemeManager( int xRes, int yRes ) pScheme = &tmpSchemes[currentScheme]; hasFgColor = hasBgColor = hasArmedFgColor = hasArmedBgColor = hasMouseDownFgColor = hasMouseDownBgColor = false; - strncpy( pScheme->schemeName, paramValue, CScheme::SCHEME_NAME_LENGTH ); + strncpy( pScheme->schemeName, paramValue, CScheme::SCHEME_NAME_LENGTH - 1 ); pScheme->schemeName[CScheme::SCHEME_NAME_LENGTH-1] = '\0'; // ensure null termination of string } @@ -291,7 +291,7 @@ CSchemeManager::CSchemeManager( int xRes, int yRes ) // pull the data out into the scheme if ( !stricmp(paramName, "FontName") ) { - strncpy( pScheme->fontName, paramValue, CScheme::FONT_NAME_LENGTH ); + strncpy( pScheme->fontName, paramValue, CScheme::FONT_NAME_LENGTH - 1 ); pScheme->fontName[CScheme::FONT_NAME_LENGTH-1] = 0; } else if ( !stricmp(paramName, "FontSize") ) diff --git a/cl_dll/vgui_ScorePanel.cpp b/cl_dll/vgui_ScorePanel.cpp index e746675d..13c2fb19 100644 --- a/cl_dll/vgui_ScorePanel.cpp +++ b/cl_dll/vgui_ScorePanel.cpp @@ -499,7 +499,7 @@ void ScorePanel::RebuildTeams() } m_iNumTeams = Q_max( j, m_iNumTeams ); - strncpy( g_TeamInfo[j].name, g_PlayerExtraInfo[i].teamname, MAX_TEAM_NAME ); + strncpy( g_TeamInfo[j].name, g_PlayerExtraInfo[i].teamname, MAX_TEAM_NAME - 1 ); g_TeamInfo[j].players = 0; } diff --git a/cl_dll/vgui_TeamFortressViewport.cpp b/cl_dll/vgui_TeamFortressViewport.cpp index 09a215b9..21a8a928 100644 --- a/cl_dll/vgui_TeamFortressViewport.cpp +++ b/cl_dll/vgui_TeamFortressViewport.cpp @@ -734,7 +734,7 @@ int TeamFortressViewport::CreateCommandMenu( const char *menuFile, int direction { // Get the mapname pfile = gEngfuncs.COM_ParseFile( pfile, token ); - strncpy( szMap, token, MAX_MAPNAME ); + strncpy( szMap, token, MAX_MAPNAME - 1 ); szMap[MAX_MAPNAME - 1] = '\0'; // Get the next token @@ -1243,7 +1243,7 @@ void TeamFortressViewport::UpdateSpectatorPanel() // create player & health string if( player && name ) { - strncpy( bottomText, name, sizeof(bottomText) ); + strncpy( bottomText, name, sizeof(bottomText) - 1 ); bottomText[ sizeof(bottomText) - 1 ] = 0; pBottomText = bottomText; } @@ -1434,7 +1434,7 @@ CMenuPanel *TeamFortressViewport::CreateTextWindow( int iTextToShow ) cText = pfile; - strncpy( cTitle, m_sMapName, MAX_TITLE_LENGTH ); + strncpy( cTitle, m_sMapName, MAX_TITLE_LENGTH - 1 ); cTitle[MAX_TITLE_LENGTH - 1] = 0; } else if( iTextToShow == SHOW_SPECHELP ) @@ -2038,7 +2038,7 @@ int TeamFortressViewport::MsgFunc_ServerName( const char *pszName, int iSize, vo { BEGIN_READ( pbuf, iSize ); - strncpy( m_szServerName, READ_STRING(), sizeof(m_szServerName) ); + strncpy( m_szServerName, READ_STRING(), sizeof(m_szServerName) - 1 ); m_szServerName[sizeof(m_szServerName) - 1] = 0; return 1; diff --git a/cl_dll/vgui_TeamFortressViewport.h b/cl_dll/vgui_TeamFortressViewport.h index 27f3c991..023d716d 100644 --- a/cl_dll/vgui_TeamFortressViewport.h +++ b/cl_dll/vgui_TeamFortressViewport.h @@ -846,7 +846,7 @@ protected: public: CMenuHandler_SpectateFollow( char *player ) { - strncpy( m_szplayer, player, MAX_COMMAND_SIZE); + strncpy( m_szplayer, player, MAX_COMMAND_SIZE - 1 ); m_szplayer[MAX_COMMAND_SIZE-1] = '\0'; } diff --git a/dlls/client.cpp b/dlls/client.cpp index e81e6772..cd33bd20 100644 --- a/dlls/client.cpp +++ b/dlls/client.cpp @@ -625,8 +625,8 @@ void ClientCommand( edict_t *pEntity ) // check the length of the command (prevents crash) // max total length is 192 ...and we're adding a string below ("Unknown command: %s\n") - strncpy( command, pcmd, 127 ); - command[127] = '\0'; + strncpy( command, pcmd, sizeof(command) - 1); + command[sizeof(command) - 1] = '\0'; // tell the user they entered an unknown command ClientPrint( &pEntity->v, HUD_PRINTCONSOLE, UTIL_VarArgs( "Unknown command: %s\n", command ) ); diff --git a/dlls/teamplay_gamerules.cpp b/dlls/teamplay_gamerules.cpp index 6f53582a..5c9a98d3 100644 --- a/dlls/teamplay_gamerules.cpp +++ b/dlls/teamplay_gamerules.cpp @@ -44,7 +44,8 @@ CHalfLifeTeamplay::CHalfLifeTeamplay() m_szTeamList[0] = 0; // Cache this because the team code doesn't want to deal with changing this in the middle of a game - strncpy( m_szTeamList, teamlist.string, TEAMPLAY_TEAMLISTLENGTH ); + strncpy( m_szTeamList, teamlist.string, TEAMPLAY_TEAMLISTLENGTH - 1 ); + m_szTeamList[TEAMPLAY_TEAMLISTLENGTH - 1] = '\0'; edict_t *pWorld = INDEXENT( 0 ); if( pWorld && pWorld->v.team ) @@ -54,7 +55,8 @@ CHalfLifeTeamplay::CHalfLifeTeamplay() const char *pTeamList = STRING( pWorld->v.team ); if( pTeamList && pTeamList[0] != '\0' ) { - strncpy( m_szTeamList, pTeamList, TEAMPLAY_TEAMLISTLENGTH ); + strncpy( m_szTeamList, pTeamList, TEAMPLAY_TEAMLISTLENGTH - 1 ); + m_szTeamList[TEAMPLAY_TEAMLISTLENGTH - 1] = '\0'; } } } @@ -183,7 +185,8 @@ const char *CHalfLifeTeamplay::SetDefaultPlayerTeam( CBasePlayer *pPlayer ) { // copy out the team name from the model char *mdls = g_engfuncs.pfnInfoKeyValue( g_engfuncs.pfnGetInfoKeyBuffer( pPlayer->edict() ), "model" ); - strncpy( pPlayer->m_szTeamName, mdls, TEAM_NAME_LENGTH ); + strncpy( pPlayer->m_szTeamName, mdls, TEAM_NAME_LENGTH - 1 ); + pPlayer->m_szTeamName[TEAM_NAME_LENGTH - 1] = '\0'; RecountTeams(); @@ -200,7 +203,8 @@ const char *CHalfLifeTeamplay::SetDefaultPlayerTeam( CBasePlayer *pPlayer ) { pTeamName = TeamWithFewestPlayers(); } - strncpy( pPlayer->m_szTeamName, pTeamName, TEAM_NAME_LENGTH ); + strncpy( pPlayer->m_szTeamName, pTeamName, TEAM_NAME_LENGTH - 1 ); + pPlayer->m_szTeamName[TEAM_NAME_LENGTH - 1] = '\0'; } return pPlayer->m_szTeamName; @@ -287,8 +291,10 @@ void CHalfLifeTeamplay::ChangePlayerTeam( CBasePlayer *pPlayer, const char *pTea // copy out the team name from the model if( pPlayer->m_szTeamName != pTeamName ) - strncpy( pPlayer->m_szTeamName, pTeamName, TEAM_NAME_LENGTH ); - + { + strncpy( pPlayer->m_szTeamName, pTeamName, TEAM_NAME_LENGTH - 1 ); + pPlayer->m_szTeamName[TEAM_NAME_LENGTH - 1] = '\0'; + } g_engfuncs.pfnSetClientKeyValue( clientIndex, g_engfuncs.pfnGetInfoKeyBuffer( pPlayer->edict() ), "model", pPlayer->m_szTeamName ); g_engfuncs.pfnSetClientKeyValue( clientIndex, g_engfuncs.pfnGetInfoKeyBuffer( pPlayer->edict() ), "team", pPlayer->m_szTeamName ); @@ -603,7 +609,8 @@ void CHalfLifeTeamplay::RecountTeams( bool bResendInfo ) tm = num_teams; num_teams++; team_scores[tm] = 0; - strncpy( team_names[tm], pTeamName, MAX_TEAMNAME_LENGTH ); + strncpy( team_names[tm], pTeamName, MAX_TEAMNAME_LENGTH - 1 ); + team_names[tm][MAX_TEAMNAME_LENGTH - 1] = '\0'; } }