Merge pull request #7580 from Robyt3/Message-Unpack-Check

Add missing unpacker error checks for server and client packets
This commit is contained in:
heinrich5991 2023-12-05 19:01:33 +00:00 committed by GitHub
commit 801274ac63
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 228 additions and 184 deletions

View file

@ -1329,7 +1329,6 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
SHA256_DIGEST *pMapSha256 = (SHA256_DIGEST *)Unpacker.GetRaw(sizeof(*pMapSha256));
int MapCrc = Unpacker.GetInt();
int MapSize = Unpacker.GetInt();
if(Unpacker.Error())
{
return;
@ -1356,7 +1355,7 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
}
int Version = Unpacker.GetInt();
int Flags = Unpacker.GetInt();
if(Version <= 0)
if(Unpacker.Error() || Version <= 0)
{
return;
}
@ -1377,37 +1376,33 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
const char *pMap = Unpacker.GetString(CUnpacker::SANITIZE_CC | CUnpacker::SKIP_START_WHITESPACES);
int MapCrc = Unpacker.GetInt();
int MapSize = Unpacker.GetInt();
const char *pError = 0;
if(Unpacker.Error())
if(Unpacker.Error() || MapSize < 0)
{
return;
if(m_DummyConnected)
DummyDisconnect(0);
}
for(int i = 0; pMap[i]; i++) // protect the player from nasty map names
{
if(pMap[i] == '/' || pMap[i] == '\\')
pError = "strange character in map name";
{
return;
}
}
if(MapSize < 0)
pError = "invalid map size";
if(pError)
DisconnectWithReason(pError);
else
if(m_DummyConnected)
{
SHA256_DIGEST *pMapSha256 = 0;
DummyDisconnect(0);
}
SHA256_DIGEST *pMapSha256 = nullptr;
const char *pMapUrl = nullptr;
if(MapDetailsWerePresent && str_comp(m_aMapDetailsName, pMap) == 0 && m_MapDetailsCrc == MapCrc)
{
pMapSha256 = &m_MapDetailsSha256;
pMapUrl = m_aMapDetailsUrl[0] ? m_aMapDetailsUrl : nullptr;
}
pError = LoadMapSearch(pMap, pMapSha256, MapCrc);
if(!pError)
if(LoadMapSearch(pMap, pMapSha256, MapCrc) != nullptr)
{
m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "client/network", "loading done");
SetLoadingStateDetail(IClient::LOADING_STATE_DETAIL_SENDING_READY);
@ -1455,6 +1450,7 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
Engine()->AddJob(m_pMapdownloadTask);
}
else
{
SendMapRequest();
}
}
@ -1466,10 +1462,10 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
int Chunk = Unpacker.GetInt();
int Size = Unpacker.GetInt();
const unsigned char *pData = Unpacker.GetRaw(Size);
// check for errors
if(Unpacker.Error() || Size <= 0 || MapCRC != m_MapdownloadCrc || Chunk != m_MapdownloadChunk || !m_MapdownloadFileTemp)
{
return;
}
io_write(m_MapdownloadFileTemp, pData, Size);
@ -1566,6 +1562,10 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
else if(Msg == NETMSG_REDIRECT)
{
int RedirectPort = Unpacker.GetInt();
if(Unpacker.Error())
{
return;
}
char aAddr[128];
char aIP[64];
NETADDR ServerAddr = ServerAddress();
@ -1578,36 +1578,48 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
const char *pName = Unpacker.GetString(CUnpacker::SANITIZE_CC);
const char *pHelp = Unpacker.GetString(CUnpacker::SANITIZE_CC);
const char *pParams = Unpacker.GetString(CUnpacker::SANITIZE_CC);
if(Unpacker.Error() == 0)
if(!Unpacker.Error())
{
m_pConsole->RegisterTemp(pName, pParams, CFGFLAG_SERVER, pHelp);
}
}
else if(Conn == CONN_MAIN && (pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && Msg == NETMSG_RCON_CMD_REM)
{
const char *pName = Unpacker.GetString(CUnpacker::SANITIZE_CC);
if(Unpacker.Error() == 0)
if(!Unpacker.Error())
{
m_pConsole->DeregisterTemp(pName);
}
}
else if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && Msg == NETMSG_RCON_AUTH_STATUS)
{
int ResultInt = Unpacker.GetInt();
if(Unpacker.Error() == 0)
if(!Unpacker.Error())
{
m_aRconAuthed[Conn] = ResultInt;
}
if(Conn == CONN_MAIN)
{
int Old = m_UseTempRconCommands;
m_UseTempRconCommands = Unpacker.GetInt();
if(Unpacker.Error() != 0)
if(Unpacker.Error())
{
m_UseTempRconCommands = 0;
}
if(Old != 0 && m_UseTempRconCommands == 0)
{
m_pConsole->DeregisterTempAll();
}
}
}
else if(!Dummy && (pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && Msg == NETMSG_RCON_LINE)
{
const char *pLine = Unpacker.GetString();
if(Unpacker.Error() == 0)
if(!Unpacker.Error())
{
GameClient()->OnRconLine(pLine);
}
}
else if(Conn == CONN_MAIN && Msg == NETMSG_PING_REPLY)
{
char aBuf[256];
@ -1618,6 +1630,11 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
{
int InputPredTick = Unpacker.GetInt();
int TimeLeft = Unpacker.GetInt();
if(Unpacker.Error())
{
return;
}
int64_t Now = time_get();
// adjust our prediction time
@ -1637,16 +1654,20 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
}
else if(Msg == NETMSG_SNAP || Msg == NETMSG_SNAPSINGLE || Msg == NETMSG_SNAPEMPTY)
{
int GameTick = Unpacker.GetInt();
int DeltaTick = GameTick - Unpacker.GetInt();
// only allow packets from the server we actually want
if(net_addr_comp(&pPacket->m_Address, &ServerAddress()))
{
return;
}
// we are not allowed to process snapshot yet
if(State() < IClient::STATE_LOADING)
{
return;
}
int GameTick = Unpacker.GetInt();
int DeltaTick = GameTick - Unpacker.GetInt();
int NumParts = 1;
int Part = 0;
@ -1665,9 +1686,10 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
}
const char *pData = (const char *)Unpacker.GetRaw(PartSize);
if(Unpacker.Error() || NumParts < 1 || NumParts > CSnapshot::MAX_PARTS || Part < 0 || Part >= NumParts || PartSize < 0 || PartSize > MAX_SNAPSHOT_PACKSIZE)
{
return;
}
// Check m_aAckGameTick to see if we already got a snapshot for that tick
if(GameTick >= m_aCurrentRecvTick[Conn] && GameTick > m_aAckGameTick[Conn])
@ -1917,12 +1939,13 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
else if(Conn == CONN_MAIN && Msg == NETMSG_RCONTYPE)
{
bool UsernameReq = Unpacker.GetInt() & 1;
if(!Unpacker.Error())
{
GameClient()->OnRconType(UsernameReq);
}
}
else
{
if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0)
}
else if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0)
{
// game message
if(!Dummy)
@ -1935,7 +1958,6 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
GameClient()->OnMessage(Msg, &Unpacker, Conn, Dummy);
}
}
}
int CClient::UnpackAndValidateSnapshot(CSnapshot *pFrom, CSnapshot *pTo)
{

View file

@ -1540,7 +1540,7 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
CUuid *pConnectionID = (CUuid *)Unpacker.GetRaw(sizeof(*pConnectionID));
int DDNetVersion = Unpacker.GetInt();
const char *pDDNetVersionStr = Unpacker.GetString(CUnpacker::SANITIZE_CC);
if(Unpacker.Error() || !str_utf8_check(pDDNetVersionStr) || DDNetVersion < 0)
if(Unpacker.Error() || DDNetVersion < 0)
{
return;
}
@ -1557,7 +1557,7 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && (m_aClients[ClientID].m_State == CClient::STATE_PREAUTH || m_aClients[ClientID].m_State == CClient::STATE_AUTH))
{
const char *pVersion = Unpacker.GetString(CUnpacker::SANITIZE_CC);
if(!str_utf8_check(pVersion))
if(Unpacker.Error())
{
return;
}
@ -1571,7 +1571,7 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
}
const char *pPassword = Unpacker.GetString(CUnpacker::SANITIZE_CC);
if(!str_utf8_check(pPassword))
if(Unpacker.Error())
{
return;
}
@ -1610,6 +1610,10 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
}
int Chunk = Unpacker.GetInt();
if(Unpacker.Error())
{
return;
}
if(Chunk != m_aClients[ClientID].m_NextMapChunk || !Config()->m_SvFastDownload)
{
SendMapData(ClientID, Chunk);
@ -1675,14 +1679,15 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
}
else if(Msg == NETMSG_INPUT)
{
m_aClients[ClientID].m_LastAckedSnapshot = Unpacker.GetInt();
const int LastAckedSnapshot = Unpacker.GetInt();
int IntendedTick = Unpacker.GetInt();
int Size = Unpacker.GetInt();
// check for errors
if(Unpacker.Error() || Size / 4 > MAX_INPUT_SIZE || IntendedTick < MIN_TICK || IntendedTick >= MAX_TICK)
{
return;
}
m_aClients[ClientID].m_LastAckedSnapshot = LastAckedSnapshot;
if(m_aClients[ClientID].m_LastAckedSnapshot > 0)
m_aClients[ClientID].m_SnapRate = CClient::SNAPRATE_FULL;
@ -1712,7 +1717,13 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
pInput->m_GameTick = IntendedTick;
for(int i = 0; i < Size / 4; i++)
{
pInput->m_aData[i] = Unpacker.GetInt();
}
if(Unpacker.Error())
{
return;
}
GameServer()->OnClientPrepareInput(ClientID, pInput->m_aData);
mem_copy(m_aClients[ClientID].m_LatestInput.m_aData, pInput->m_aData, MAX_INPUT_SIZE * sizeof(int));
@ -1727,11 +1738,11 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
else if(Msg == NETMSG_RCON_CMD)
{
const char *pCmd = Unpacker.GetString();
if(!str_utf8_check(pCmd))
if(Unpacker.Error())
{
return;
}
if(Unpacker.Error() == 0 && !str_comp(pCmd, "crashmeplx"))
if(!str_comp(pCmd, "crashmeplx"))
{
int Version = m_aClients[ClientID].m_DDNetVersion;
if(GameServer()->PlayerExists(ClientID) && Version < VERSION_DDNET_OLD)
@ -1739,7 +1750,7 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
m_aClients[ClientID].m_DDNetVersion = VERSION_DDNET_OLD;
}
}
else if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && Unpacker.Error() == 0 && m_aClients[ClientID].m_Authed)
else if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && m_aClients[ClientID].m_Authed)
{
if(GameServer()->PlayerExists(ClientID))
{
@ -1762,17 +1773,21 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
}
else if(Msg == NETMSG_RCON_AUTH)
{
if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) == 0)
{
return;
}
const char *pName = "";
if(!IsSixup(ClientID))
{
pName = Unpacker.GetString(CUnpacker::SANITIZE_CC); // login name, now used
}
const char *pPw = Unpacker.GetString(CUnpacker::SANITIZE_CC);
if(!str_utf8_check(pPw) || !str_utf8_check(pName))
if(Unpacker.Error())
{
return;
}
if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && Unpacker.Error() == 0)
{
int AuthLevel = -1;
int KeySlot = -1;
@ -1812,9 +1827,11 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
m_aClients[ClientID].m_Authed = AuthLevel; // Keeping m_Authed around is unwise...
m_aClients[ClientID].m_AuthKey = KeySlot;
int SendRconCmds = IsSixup(ClientID) ? true : Unpacker.GetInt();
if(Unpacker.Error() == 0 && SendRconCmds)
if(!Unpacker.Error() && SendRconCmds)
{
// AUTHED_ADMIN - AuthLevel gets the proper IConsole::ACCESS_LEVEL_<x>
m_aClients[ClientID].m_pRconCmdToSend = Console()->FirstCommandInfo(AUTHED_ADMIN - AuthLevel, CFGFLAG_SERVER);
}
char aBuf[256];
const char *pIdent = m_AuthManager.KeyIdent(KeySlot);
@ -1864,7 +1881,6 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
SendRconLine(ClientID, "Wrong password.");
}
}
}
else if(Msg == NETMSG_PING)
{
CMsgPacker Msgp(NETMSG_PING_REPLY, true);
@ -1896,10 +1912,9 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
}
}
}
else
else if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && m_aClients[ClientID].m_State >= CClient::STATE_READY)
{
// game message
if((pPacket->m_Flags & NET_CHUNKFLAG_VITAL) != 0 && m_aClients[ClientID].m_State >= CClient::STATE_READY)
GameServer()->OnMessage(Msg, &Unpacker, ClientID);
}
}
@ -2491,13 +2506,14 @@ void CServer::PumpNetwork(bool PacketWaiting)
Unpacker.Reset((unsigned char *)Packet.m_pData + sizeof(SERVERBROWSE_GETINFO), Packet.m_DataSize - sizeof(SERVERBROWSE_GETINFO));
int SrvBrwsToken = Unpacker.GetInt();
if(Unpacker.Error())
{
continue;
}
CPacker Packer;
CNetChunk Response;
GetServerInfoSixup(&Packer, SrvBrwsToken, RateLimitServerInfoConnless());
CNetChunk Response;
Response.m_ClientID = -1;
Response.m_Address = Packet.m_Address;
Response.m_Flags = NETSENDFLAG_CONNLESS;

View file

@ -172,6 +172,12 @@ const char *CUnpacker::GetString(int SanitizeType)
}
m_pCurrent++;
if(!str_utf8_check(pPtr))
{
m_Error = true;
return "";
}
// sanitize all strings
if(SanitizeType & SANITIZE)
str_sanitize(pPtr);