Fix possible overflow errors on leveling up beyond int64_t limit

- added separate giveExperience method instead of weird changePrimSkill
- experience is now always used in form of int64_t
- max supported level reduced from 201 to 197 to fit into int64_t
- fixed undefined behavior in experience calculation

client/Client.h

@@ -164,6 +164,7 @@ public:
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override {return false;};
 	void createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype) override {};
 	void setOwner(const CGObjectInstance * obj, PlayerColor owner) override {};
+	void giveExperience(const CGHeroInstance * hero, TExpType val) override {};
 	void changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs = false) override {};
 	void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs = false) override {};
 

lib/CHeroHandler.cpp

@@ -654,14 +654,21 @@ void CHeroHandler::loadExperience()
 	expPerLevel.push_back(24320);
 	expPerLevel.push_back(28784);
 	expPerLevel.push_back(34140);
-	while (expPerLevel[expPerLevel.size() - 1] > expPerLevel[expPerLevel.size() - 2])
+
+	for (;;)
 	{
 		auto i = expPerLevel.size() - 1;
-		auto diff = expPerLevel[i] - expPerLevel[i-1];
-		diff += diff / 5;
-		expPerLevel.push_back (expPerLevel[i] + diff);
+		auto currExp = expPerLevel[i];
+		auto prevExp = expPerLevel[i-1];
+		auto prevDiff = currExp - prevExp;
+		auto nextDiff = prevDiff + prevDiff / 5;
+		auto maxExp = std::numeric_limits<decltype(currExp)>::max();
+
+		if (currExp > maxExp - nextDiff)
+			break; // overflow point reached
+
+		expPerLevel.push_back (currExp + nextDiff);
 	}
-	expPerLevel.pop_back();//last value is broken
 }
 
 /// convert h3-style ID (e.g. Gobin Wolf Rider) to vcmi (e.g. goblinWolfRider)
@@ -741,12 +748,12 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod
 	registerObject(scope, "hero", name, object->getIndex());
 }
 
-ui32 CHeroHandler::level (ui64 experience) const
+ui32 CHeroHandler::level (TExpType experience) const
 {
 	return static_cast<ui32>(boost::range::upper_bound(expPerLevel, experience) - std::begin(expPerLevel));
 }
 
-ui64 CHeroHandler::reqExp (ui32 level) const
+TExpType CHeroHandler::reqExp (ui32 level) const
 {
 	if(!level)
 		return 0;
@@ -762,6 +769,11 @@ ui64 CHeroHandler::reqExp (ui32 level) const
 	}
 }
 
+ui32 CHeroHandler::maxSupportedLevel() const
+{
+	return expPerLevel.size();
+}
+
 std::set<HeroTypeID> CHeroHandler::getDefaultAllowed() const
 {
 	std::set<HeroTypeID> result;

lib/CHeroHandler.h

@@ -176,8 +176,8 @@ protected:
 class DLL_LINKAGE CHeroHandler : public CHandlerBase<HeroTypeID, HeroType, CHero, HeroTypeService>
 {
 	/// expPerLEvel[i] is amount of exp needed to reach level i;
-	/// consists of 201 values. Any higher levels require experience larger that ui64 can hold
-	std::vector<ui64> expPerLevel;
+	/// consists of 196 values. Any higher levels require experience larger that TExpType can hold
+	std::vector<TExpType> expPerLevel;
 
 	/// helpers for loading to avoid huge load functions
 	void loadHeroArmy(CHero * hero, const JsonNode & node) const;
@@ -191,8 +191,9 @@ class DLL_LINKAGE CHeroHandler : public CHandlerBase<HeroTypeID, HeroType, CHero
 public:
 	CHeroClassHandler classes;
 
-	ui32 level(ui64 experience) const; //calculates level corresponding to given experience amount
-	ui64 reqExp(ui32 level) const; //calculates experience required for given level
+	ui32 level(TExpType experience) const; //calculates level corresponding to given experience amount
+	TExpType reqExp(ui32 level) const; //calculates experience required for given level
+	ui32 maxSupportedLevel() const;
 
 	std::vector<JsonNode> loadLegacyData() override;
 

lib/IGameCallback.h

@@ -84,6 +84,7 @@ public:
 	virtual bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) = 0;
 	virtual void createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype) = 0;
 	virtual void setOwner(const CGObjectInstance * objid, PlayerColor owner)=0;
+	virtual void giveExperience(const CGHeroInstance * hero, TExpType val) =0;
 	virtual void changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs=false)=0;
 	virtual void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs=false)=0;
 	virtual void showBlockingDialog(BlockingDialog *iw) =0;

lib/mapObjects/CGHeroInstance.cpp

@@ -1439,7 +1439,7 @@ void CGHeroInstance::setPrimarySkill(PrimarySkill primarySkill, si64 value, ui8
 
 bool CGHeroInstance::gainsLevel() const
 {
-	return exp >= static_cast<TExpType>(VLC->heroh->reqExp(level+1));
+	return level < VLC->heroh->maxSupportedLevel() && exp >= static_cast<TExpType>(VLC->heroh->reqExp(level+1));
 }
 
 void CGHeroInstance::levelUp(const std::vector<SecondarySkill> & skills)

lib/mapObjects/CGTownBuilding.cpp

@@ -249,8 +249,12 @@ void CTownBonus::onHeroVisit (const CGHeroInstance * h) const
 			iw.player = cb->getOwner(heroID);
 				iw.text.appendRawString(getVisitingBonusGreeting());
 			cb->showInfoDialog(&iw);
-			cb->changePrimSkill (cb->getHero(heroID), what, val);
-				town->addHeroToStructureVisitors(h, indexOnTV);
+			if (what == PrimarySkill::EXPERIENCE)
+				cb->giveExperience(cb->getHero(heroID), val);
+			else
+				cb->changePrimSkill(cb->getHero(heroID), what, val);
+
+			town->addHeroToStructureVisitors(h, indexOnTV);
 		}
 	}
 }

lib/mapObjects/MiscObjects.cpp

@@ -1138,7 +1138,7 @@ void CGSirens::onHeroVisit( const CGHeroInstance * h ) const
 			xp = h->calculateXp(static_cast<int>(xp));
 			iw.text.appendLocalString(EMetaText::ADVOB_TXT,132);
 			iw.text.replaceNumber(static_cast<int>(xp));
-			cb->changePrimSkill(h, PrimarySkill::EXPERIENCE, xp, false);
+			cb->giveExperience(h, xp);
 		}
 		else
 		{

lib/rewardable/Interface.cpp

@@ -117,7 +117,7 @@ void Rewardable::Interface::grantRewardBeforeLevelup(IGameCallback * cb, const R
 	for(int i=0; i< info.reward.primary.size(); i++)
 		cb->changePrimSkill(hero, static_cast<PrimarySkill>(i), info.reward.primary[i], false);
 
-	si64 expToGive = 0;
+	TExpType expToGive = 0;
 
 	if (info.reward.heroLevel > 0)
 		expToGive += VLC->heroh->reqExp(hero->level+info.reward.heroLevel) - VLC->heroh->reqExp(hero->level);
@@ -126,7 +126,7 @@ void Rewardable::Interface::grantRewardBeforeLevelup(IGameCallback * cb, const R
 		expToGive += hero->calculateXp(info.reward.heroExperience);
 
 	if(expToGive)
-		cb->changePrimSkill(hero, PrimarySkill::EXPERIENCE, expToGive);
+		cb->giveExperience(hero, expToGive);
 }
 
 void Rewardable::Interface::grantRewardAfterLevelup(IGameCallback * cb, const Rewardable::VisitInfo & info, const CArmedInstance * army, const CGHeroInstance * hero) const

server/CGameHandler.cpp

@@ -365,52 +365,60 @@ void CGameHandler::expGiven(const CGHeroInstance *hero)
 // 		levelUpHero(hero);
 }
 
-void CGameHandler::changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs)
+void CGameHandler::giveExperience(const CGHeroInstance * hero, TExpType amountToGain)
 {
-	if (which == PrimarySkill::EXPERIENCE) // Check if scenario limit reached
+	TExpType maxExp = VLC->heroh->reqExp(VLC->heroh->maxSupportedLevel());
+	TExpType currExp = hero->exp;
+
+	if (gs->map->levelLimit != 0)
+		maxExp = VLC->heroh->reqExp(gs->map->levelLimit);
+
+	TExpType canGainExp = 0;
+	if (maxExp > currExp)
+		canGainExp = maxExp - currExp;
+
+	if (amountToGain > canGainExp)
 	{
-		if (gs->map->levelLimit != 0)
-		{
-			TExpType expLimit = VLC->heroh->reqExp(gs->map->levelLimit);
-			TExpType resultingExp = abs ? val : hero->exp + val;
-			if (resultingExp > expLimit)
-			{
-				// set given experience to max possible, but don't decrease if hero already over top
-				abs = true;
-				val = std::max(expLimit, hero->exp);
+		// set given experience to max possible, but don't decrease if hero already over top
+		amountToGain = canGainExp;
 
-				InfoWindow iw;
-				iw.player = hero->tempOwner;
-				iw.text.appendLocalString(EMetaText::GENERAL_TXT, 1); //can gain no more XP
-				iw.text.replaceRawString(hero->getNameTranslated());
-				sendAndApply(&iw);
-			}
-		}
+		InfoWindow iw;
+		iw.player = hero->tempOwner;
+		iw.text.appendLocalString(EMetaText::GENERAL_TXT, 1); //can gain no more XP
+		iw.text.replaceRawString(hero->getNameTranslated());
+		sendAndApply(&iw);
 	}
 
 	SetPrimSkill sps;
 	sps.id = hero->id;
-	sps.which = which;
-	sps.abs = abs;
-	sps.val = val;
+	sps.which = PrimarySkill::EXPERIENCE;
+	sps.abs = false;
+	sps.val = amountToGain;
 	sendAndApply(&sps);
 
-	//only for exp - hero may level up
-	if (which == PrimarySkill::EXPERIENCE)
+	//hero may level up
+	if (hero->commander && hero->commander->alive)
 	{
-		if (hero->commander && hero->commander->alive)
-		{
-			//FIXME: trim experience according to map limit?
-			SetCommanderProperty scp;
-			scp.heroid = hero->id;
-			scp.which = SetCommanderProperty::EXPERIENCE;
-			scp.amount = val;
-			sendAndApply (&scp);
-			CBonusSystemNode::treeHasChanged();
-		}
-
-		expGiven(hero);
+		//FIXME: trim experience according to map limit?
+		SetCommanderProperty scp;
+		scp.heroid = hero->id;
+		scp.which = SetCommanderProperty::EXPERIENCE;
+		scp.amount = amountToGain;
+		sendAndApply (&scp);
+		CBonusSystemNode::treeHasChanged();
 	}
+
+	expGiven(hero);
+}
+
+void CGameHandler::changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs)
+{
+	SetPrimSkill sps;
+	sps.id = hero->id;
+	sps.which = which;
+	sps.abs = abs;
+	sps.val = val;
+	sendAndApply(&sps);
 }
 
 void CGameHandler::changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs)
@@ -658,7 +666,7 @@ void CGameHandler::onNewTurn()
 		{
 			if (obj && obj->ID == Obj::PRISON) //give imprisoned hero 0 exp to level him up. easiest to do at this point
 			{
-				changePrimSkill (getHero(obj->id), PrimarySkill::EXPERIENCE, 0);
+				giveExperience(getHero(obj->id), 0);
 			}
 		}
 	}
@@ -3708,7 +3716,7 @@ bool CGameHandler::sacrificeCreatures(const IMarket * market, const CGHeroInstan
 	int expSum = 0;
 	auto finish = [this, &hero, &expSum]()
 	{
-		changePrimSkill(hero, PrimarySkill::EXPERIENCE, hero->calculateXp(expSum));
+		giveExperience(hero, hero->calculateXp(expSum));
 	};
 
 	for(int i = 0; i < slot.size(); ++i)
@@ -3749,7 +3757,7 @@ bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * h
 	int expSum = 0;
 	auto finish = [this, &hero, &expSum]()
 	{
-		changePrimSkill(hero, PrimarySkill::EXPERIENCE, hero->calculateXp(expSum));
+		giveExperience(hero, hero->calculateXp(expSum));
 	};
 
 	for(int i = 0; i < slot.size(); ++i)

server/CGameHandler.h

@@ -102,6 +102,7 @@ public:
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override;
 	void createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype) override;
 	void setOwner(const CGObjectInstance * obj, PlayerColor owner) override;
+	void giveExperience(const CGHeroInstance * hero, TExpType val) override;
 	void changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs=false) override;
 	void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs=false) override;
 

server/battles/BattleResultProcessor.cpp

@@ -494,7 +494,7 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 	}
 	//give exp
 	if(!finishingBattle->isDraw() && battleResult->exp[finishingBattle->winnerSide] && finishingBattle->winnerHero)
-		gameHandler->changePrimSkill(finishingBattle->winnerHero, PrimarySkill::EXPERIENCE, battleResult->exp[finishingBattle->winnerSide]);
+		gameHandler->giveExperience(finishingBattle->winnerHero, battleResult->exp[finishingBattle->winnerSide]);
 
 	BattleResultAccepted raccepted;
 	raccepted.battleID = battle.getBattle()->getBattleID();

server/processors/PlayerMessageProcessor.cpp

@@ -261,7 +261,7 @@ void PlayerMessageProcessor::cheatLevelup(PlayerColor player, const CGHeroInstan
 		levelsToGain = 1;
 	}
 
-	gameHandler->changePrimSkill(hero, PrimarySkill::EXPERIENCE, VLC->heroh->reqExp(hero->level + levelsToGain) - VLC->heroh->reqExp(hero->level));
+	gameHandler->giveExperience(hero, VLC->heroh->reqExp(hero->level + levelsToGain) - VLC->heroh->reqExp(hero->level));
 }
 
 void PlayerMessageProcessor::cheatExperience(PlayerColor player, const CGHeroInstance * hero, std::vector<std::string> words)
@@ -280,7 +280,7 @@ void PlayerMessageProcessor::cheatExperience(PlayerColor player, const CGHeroIns
 		expAmountProcessed = 10000;
 	}
 
-	gameHandler->changePrimSkill(hero, PrimarySkill::EXPERIENCE, expAmountProcessed);
+	gameHandler->giveExperience(hero, expAmountProcessed);
 }
 
 void PlayerMessageProcessor::cheatMovement(PlayerColor player, const CGHeroInstance * hero, std::vector<std::string> words)