Fix quest array off-by-ones and add QuestID bounds checks

Several quest-loading loops used <=N over arrays sized N (Variables,
Switches, Items, EpisodeVar, JobVar, PlanetVar, UnionVar), writing one
element past the end on every character load. Also guard STB_QUEST
table lookups with a rowcount bounds check before indexing by
QuestID, which could otherwise read out of bounds for stale/invalid
quest ids.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ThomasandClaude Sonnet 5 committed 2026-07-09 06:35:11 +02:00
1 parent 725620e23c
commit f59158a95c
4 files changed
+13 -12

No files matched your search

+1 -1
View File
@@ -1327,7 +1327,7 @@ struct SQuest
void AddItem(CItem* item, byte btOp = 2)
{
for(dword i = 0; i < 6; i++)
for(dword i = 0; i < 5; i++)
{
if(Items[i].GetPakHeader() == item->GetPakHeader())
{
+9 -8
View File
@@ -78,6 +78,7 @@ void DecodeBinary( char* encoded, unsigned char* data )
bool CPlayer::loaddata( )
{
//Log(MSG_INFO,"Loading character data");
memset( &quest, 0, sizeof(quest) ); // quest slots without a saved row must default to zero, not stack/heap garbage
MYSQL_ROW row;
MYSQL_RES *result = GServer->DB->QStore("SELECT level,face,hairStyle,sex,classid,zuly,str,dex,_int,con,cha,sen,curHp,curMp,id,statp,skillp,exp,stamina,quickbar,basic_skills,askill,askilllvl,pskill,pskilllvl,respawnid,clanid,clan_rank,townid,isgm,isdev FROM characters WHERE char_name='%s'", CharInfo->charname);
if(result==NULL) return false;
@@ -577,7 +578,7 @@ bool CPlayer::loaddata( )
quest.quests[Index].StartTime = atoi(row[1]);
tmpid = strtok( row[2] , "|");
for(unsigned int j=0;j<=10;j++) //quest variables
for(unsigned int j=0;j<10;j++) //quest variables
{
if(tmpid != NULL)
quest.quests[Index].Variables[j] = atoi(tmpid);
@@ -587,7 +588,7 @@ bool CPlayer::loaddata( )
//Log( MSG_DEBUG, "Quest Var[%i][%i] = %i",Index ,j ,quest.quests[Index].Variables[j]);
}
tmpid = strtok( row[3] , "|"); //quest switches
for(unsigned int j=0;j<=4;j++)
for(unsigned int j=0;j<4;j++)
{
if(tmpid != NULL)
quest.quests[Index].Switches[j] = atoi(tmpid);
@@ -597,7 +598,7 @@ bool CPlayer::loaddata( )
//Log( MSG_DEBUG, "Quest Switch[%i][%i] = %i",Index ,j ,quest.quests[Index].Switches[j]);
}
tmpid = strtok( row[4] , "|"); //quest items
for(unsigned int j=0;j<=5;j++)
for(unsigned int j=0;j<5;j++)
{
if(tmpid != NULL)
{
@@ -614,7 +615,7 @@ bool CPlayer::loaddata( )
//Log( MSG_DEBUG, "Quest Item[%i][%i] = %i",Index , j ,quest.quests[Index].Items[j].itemnum);
}
tmpid = strtok( row[5] , "|"); //quest item count
for(unsigned int j=0;j<=5;j++)
for(unsigned int j=0;j<5;j++)
{
if(tmpid != NULL)
quest.quests[Index].Items[j].count = atoi(tmpid);
@@ -624,7 +625,7 @@ bool CPlayer::loaddata( )
//Log( MSG_DEBUG, "Quest Item Count[%i][%i] = %i",Index ,j ,quest.quests[Index].Items[j].count);
}
tmpid = strtok( row[6] , "|");
for(unsigned int j=0;j<=5;j++) //episode variables
for(unsigned int j=0;j<5;j++) //episode variables
{
if(tmpid != NULL)
quest.EpisodeVar[j] = atoi(tmpid);
@@ -634,7 +635,7 @@ bool CPlayer::loaddata( )
//Log( MSG_DEBUG, "Quest EpisodeVar[%i][%i] = %i",Index ,j ,quest.EpisodeVar[j]);
}
tmpid = strtok( row[7] , "|"); //job variables
for(unsigned int j=0;j<=3;j++)
for(unsigned int j=0;j<3;j++)
{
if(tmpid != NULL)
quest.JobVar[j] = atoi(tmpid);
@@ -644,7 +645,7 @@ bool CPlayer::loaddata( )
//Log( MSG_DEBUG, "Quest JobVar[%i][%i] = %i",Index ,j ,quest.JobVar[j]);
}
tmpid = strtok( row[8] , "|"); //Planet variables
for(unsigned int j=0;j<=7;j++)
for(unsigned int j=0;j<7;j++)
{
if(tmpid != NULL)
quest.PlanetVar[j] = atoi(tmpid);
@@ -653,7 +654,7 @@ bool CPlayer::loaddata( )
tmpid = strtok(NULL, ",|");
}
tmpid = strtok( row[9] , "|"); //Planet variables
for(unsigned int j=0;j<=10;j++)
for(unsigned int j=0;j<10;j++)
{
if(tmpid != NULL)
quest.UnionVar[j] = atoi(tmpid);
+1 -1
View File
@@ -288,7 +288,7 @@ QUESTCOND(008)
server->SendPM(client, "QuestTime - ulTime: %i btOp: %i", data->ulTime, data->btOp);
if (client->CheckQuest < 0) return QUEST_FAILURE; // Not checking a quest, don't return success
long int Time = 0;
if (server->STB_QUEST.rows[client->quest.quests[client->CheckQuest].QuestID][1] > 0)
if (client->quest.quests[client->CheckQuest].QuestID < server->STB_QUEST.rowcount && server->STB_QUEST.rows[client->quest.quests[client->CheckQuest].QuestID][1] > 0)
{
Time += client->quest.quests[client->CheckQuest].StartTime; // Start time
Time += server->STB_QUEST.rows[client->quest.quests[client->CheckQuest].QuestID][1] * 10; // Time to finish
+2 -2
View File
@@ -169,7 +169,7 @@ void CWorldServer::pakQuestData( CPlayer *thisclient )
//Log( MSG_DEBUG, "Initialized Time");
//Log( MSG_DEBUG, "This Quest quest id = %i",thisclient->quest.quests[i].QuestID);
//Log( MSG_DEBUG, "This Quest STB quest id = %i",STB_QUEST.rows[thisclient->quest.quests[i].QuestID][1]);
if (thisclient->quest.quests[i].QuestID > 0 && STB_QUEST.rows[thisclient->quest.quests[i].QuestID][1] > 0)
if (thisclient->quest.quests[i].QuestID > 0 && thisclient->quest.quests[i].QuestID < STB_QUEST.rowcount && STB_QUEST.rows[thisclient->quest.quests[i].QuestID][1] > 0)
{
//Log( MSG_DEBUG, "client has a timed quest");
Time += thisclient->quest.quests[i].StartTime; // Start time
@@ -187,7 +187,7 @@ void CWorldServer::pakQuestData( CPlayer *thisclient )
//Log( MSG_DEBUG, "added quest vars");
for(dword j = 0; j < 4; j++) pakout.AddByte(thisclient->quest.quests[i].Switches[j]);
//Log( MSG_DEBUG, "added quest switches");
for(dword j = 0; j < 6; j++)
for(dword j = 0; j < 5; j++)
{
if (thisclient->quest.quests[i].Items[j].itemnum != 0)
{