Forger Entities can forge more times than intended

mediumCode4rena

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntityForging/EntityForging.sol#L87-L90 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntityForging/EntityForging.sol#L133

Vulnerability details

Vulnerability details

As per the docs https://github.com/TraitForge/GitBook/blob/main/TraitForger%20Entity/Forge%20Potential%20.md

Entities' Forge Potential is determined by the [5] digit in entropy
Each entity may have the ability to forge 0-9 times depending on its Forge Potential.

This means an entity with forgePotential of 0 is infertile, an entity with forgePotential = 1 can forge 1 time an year, an entity with forgePotential = 2 can forge 2 times an year and so on.

The number of times an enitity forges is tracked in the forgingCounts mapping.

This mapping is incremented for both the forgerTokenId and mergerTokenId everytime a succesful forging happens thru forgeWithListed.

The forgingCounts[] of 'mergerTokenIds' is handled correctly. The increment of forgingCounts[] preceeds the condition check.

forgingCounts[mergerTokenId]++;
    require(mergerForgePotential > 0 &&
		        forgingCounts[mergerTokenId] <= mergerForgePotential,
			      'forgePotential insufficient'
);

However, Forger Entities 'forgerTokenId' are able to exceed the forging limit due to increment of forgingCounts[] happening after the condition check.

The check is made during the listForForging call. But the forgingCounts[] mapping is only incremented during forgeWithListed call.

This allows the 'Forger Entities' to list one more time than intended.

This results in, Each Forger entity with non-zero forgingPotential, having the ability to forge 2-10 times, instead of 1-9 times intended by the protocol.

Impact

As long as Forger Entities have a non-zero forgingPotential, the Entity is able to exceed the limit intended by forgingPotential.

Proof of Concept

Assume a Forger Entity of

   _tokenIds = 123 
   forgingPotential = 1 (extracted from the 5th digit of entropy).
   Initial forgingCounts for tokenId 1 is 0.

1. First Listing:

During listing : forgingCounts[123] = 0 Check: forgingCounts[123] <= forgePotential (0 <= 1) => Pass During forging : forgingCounts[123]++ => 1 (during forging)

2. Second Listing:

During listing : forgingCounts[123] = 1 Check: forgingCounts[123] <= forgePotential (1 <= 1) => Pass During forging : forgingCounts[123]++ => 2

3. Third Listing: (Reverts only at the third attempt)

During listing : forgingCounts[123] = 2 (during listing) Check: forgingCounts[123] < forgePotential (2 <= 1) => Fail During forging : forging does not proceed.

This shows, a forger entity with a forgingPotential of 1 can forge 2 times. similarly a forger entity with forgingPotential of 9 can forge 10 times.

Tools Used

Foundry, Manual Analysis

Recommended Mitigation Steps

This can be fixed in different ways. The simplest way is to use the '<' instead of the '<=' in the listForForging condition check.

forgingCounts[tokenId] < forgePotential

require(
      forgePotential > 0 && forgingCounts[tokenId] < forgePotential,
      'Entity has reached its forging limit'
      );

Assessed type

Invalid Validation