0xGreyWolf - `CouncilMembers::_retrieve()` loops over an array of `balances` to stream `individualBalance` and as the array size (council members) grow, gas cost expands until it becomes unusable. #162
CouncilMembers::_retrieve() loops over an array of balances to stream individualBalance and as the array size (council members) grow, gas cost expands until it becomes unusable.
Summary
An internal function CouncilMembers::_retrieve() loops over an array balances then adds individualBalance on every element. Every element represents nft token holders / Council Members so it is expected to increase over time. As it happens, the gas cost increases until it becomes unusable either by impracticality or until it reaches the block gas limit.
The CouncilMembers::_retrieve() is also used in multiple occasions as listed in Vulnerability Detail.
/**
* @notice Retrieve and distribute TELCOIN to council members based on the stream from _target
* @dev This function fetches the maximum possible TELCOIN and distributes it equally among all council members.
* @dev It also updates the running balance to ensure accurate distribution during subsequent calls.
*/
function _retrieve() internal {
...
// Add the individual balance to each council member's balance
for (uint i = 0; i < balances.length; i++) {
balances[i] += individualBalance;
}
}
Vulnerability Detail
As the council members increase, the array size increases because that is where the tokenId is stored. As it happens the gas cost increases until the time it is unusable either by impracticality or until it reaches the block gas limit.
This internal function is "also" used in multiple functions. It is expected that the adverse effect will spread across them too.
CouncilMembers::retrieve()
CouncilMembers::claim()
CouncilMembers::removeFromOffice()
CouncilMembers::mint()
CouncilMembers::burn()
CouncilMembers::_update()
Impact
As council members become too large, functions listed above will never be executed.
Invalid, it is extremely if not impossible that an OOG will occur given council members will not reach that kind of lengths, since it is admin approved
0xGreyWolf
high
CouncilMembers::_retrieve()
loops over an array ofbalances
to streamindividualBalance
and as the array size (council members) grow, gas cost expands until it becomes unusable.Summary
An internal function CouncilMembers::_retrieve() loops over an array
balances
then addsindividualBalance
on every element. Every element represents nft token holders / Council Members so it is expected to increase over time. As it happens, the gas cost increases until it becomes unusable either by impracticality or until it reaches the block gas limit.The
CouncilMembers::_retrieve()
is also used in multiple occasions as listed in Vulnerability Detail.Here's a quick look of the code.
Vulnerability Detail
As the council members increase, the array size increases because that is where the tokenId is stored. As it happens the gas cost increases until the time it is unusable either by impracticality or until it reaches the block gas limit.
This internal function is "also" used in multiple functions. It is expected that the adverse effect will spread across them too.
CouncilMembers::retrieve()
CouncilMembers::claim()
CouncilMembers::removeFromOffice()
CouncilMembers::mint()
CouncilMembers::burn()
CouncilMembers::_update()
Impact
Code Snippet
Tool used
Manual Review
Recommendation
Use mapping to track balances per tokenId then implement the code around it.