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.
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.
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
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
andmergerTokenId
everytime a succesful forging happens thruforgeWithListed
.The
forgingCounts[]
of 'mergerTokenIds' is handled correctly. The increment offorgingCounts[]
preceeds the condition check.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 theforgingCounts[]
mapping is only incremented duringforgeWithListed
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
During listing : forgingCounts[123] = 0 Check: forgingCounts[123] <= forgePotential (0 <= 1) => Pass During forging : forgingCounts[123]++ => 1 (during forging)
During listing : forgingCounts[123] = 1 Check: forgingCounts[123] <= forgePotential (1 <= 1) => Pass During forging : forgingCounts[123]++ => 2
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
Assessed type
Invalid Validation