Claimable assets can be lost for the claimer, if the claimer chooses to fund back the contract and redistribute the claimed amount.
Proof of Concept
In DevFund.sol, in the claim() function, users will receive their claimable amount if they have any. Since this reward received is the native currency, the user is granted control at this point. The user can transfer whatever amount that is received back to the DevFund.sol contract at this point as a way of contributing back to the dev fund.
totalRewardDebt is used to account for how much each user should receive in terms of rewards based on the weightage of each dev. However, because claim() overwrites info.rewardDebt at the end of the function, this particular sender will have their totalRewardDebt overwritten to the latest value, without the sender having claimed yet. This will result in the lost of assets based on how much they should be distributed.
For instance, if Alice has 10% weightage and Bob has 90% weightage, and total claimable fund is 1000 ETH, when Bob claims their share of asset, they receive 900 ETH. Bob decides to fund back the contract and redistribute them back right when the amount is received. The contract should compute Alice as now having 100 ETH (old) + 90 ETH (new) and Bob should have 810 ETH (new).
However, due to the described issue of totalRewardDebt being overwritten, the 810 ETH is lost forever and can never be claimed.
Setting this issue at medium severity, as it is a loss of fund, but conditioned on the claimer funding back the contract immediately when ETH is received.
Tools Used
Manual review
Recommended Mitigation Steps
Consider updating rewardDebt before transferring rewards to msg.sender.
Lines of code
https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/DevFund/DevFund.sol#L69
Vulnerability details
Impact
Claimable assets can be lost for the claimer, if the claimer chooses to fund back the contract and redistribute the claimed amount.
Proof of Concept
In
DevFund.sol
, in theclaim()
function, users will receive their claimable amount if they have any. Since this reward received is the native currency, the user is granted control at this point. The user can transfer whatever amount that is received back to theDevFund.sol
contract at this point as a way of contributing back to the dev fund.totalRewardDebt
is used to account for how much each user should receive in terms of rewards based on the weightage of each dev. However, becauseclaim()
overwritesinfo.rewardDebt
at the end of the function, this particular sender will have theirtotalRewardDebt
overwritten to the latest value, without the sender having claimed yet. This will result in the lost of assets based on how much they should be distributed.For instance, if Alice has 10% weightage and Bob has 90% weightage, and total claimable fund is 1000 ETH, when Bob claims their share of asset, they receive 900 ETH. Bob decides to fund back the contract and redistribute them back right when the amount is received. The contract should compute Alice as now having 100 ETH (old) + 90 ETH (new) and Bob should have 810 ETH (new).
However, due to the described issue of
totalRewardDebt
being overwritten, the 810 ETH is lost forever and can never be claimed.Setting this issue at medium severity, as it is a loss of fund, but conditioned on the claimer funding back the contract immediately when ETH is received.
Tools Used
Manual review
Recommended Mitigation Steps
Consider updating
rewardDebt
before transferring rewards tomsg.sender
.Assessed type
ETH-Transfer