Submission Details
Severity:
Users will lose all their previous weth rewards when the add to their stake
Summary
Whenever a user decides to add to their stake amount, the call will update their earnings without sending the previous earnings to these users if they have values to claim, thus causing these rewards to be lost and inaccessible forever.
Vulnerability Details
Stake shares doesn't not check if a user has previous claims to be collected from the vault hence this funds are lost.
function stake(uint128 vaultId, uint128 shares) external {
// to prevent safe cast overflow errors
if (shares < Constants.MIN_OF_SHARES_TO_STAKE) {
revert Errors.QuantityOfSharesLessThanTheMinimumAllowed(Constants.MIN_OF_SHARES_TO_STAKE, uint256(shares));
}
// fetch storage slot for vault by id
Vault.Data storage vault = Vault.loadLive(vaultId);
// prepare the `Vault::recalculateVaultsCreditCapacity` call
uint256[] memory vaultsIds = new uint256[]();
vaultsIds[0] = uint256(vaultId);
// updates the vault's credit capacity and perform all vault state
// transitions before updating `msg.sender` staked shares
Vault.recalculateVaultsCreditCapacity(vaultsIds);
// load distribution data
@audit>> Distribution.Data storage wethRewardDistribution = vault.wethRewardDistribution;
// cast actor address to bytes32
@audit>> bytes32 actorId = bytes32(uint256(uint160(msg.sender)));
@audit>> // accumulate the actor's pending reward before staking
@audit>> wethRewardDistribution.accumulateActor(actorId);
// load actor distribution data
Distribution.Actor storage actor = wethRewardDistribution.actor[actorId];
// calculate actor updated shares amount
UD60x18 updatedActorShares = ud60x18(actor.shares).add(ud60x18(shares));
// update actor staked shares
wethRewardDistribution.setActorShares(actorId, updatedActorShares);
// transfer shares from actor
IERC20(vault.indexToken).safeTransferFrom(msg.sender, address(this), shares);
// emit an event
emit LogStake(vaultId, msg.sender, shares);
}
Accumulation without first claiming fees will cause this actor to lose this funds.
These was correctly enforced in the unsatking action
function unstake(uint128 vaultId, uint256 shares) external {
// fetch storage slot for vault by id
Vault.Data storage vault = Vault.loadLive(vaultId);
// prepare the `Vault::recalculateVaultsCreditCapacity` call
uint256[] memory vaultsIds = new uint256[]();
vaultsIds[0] = uint256(vaultId);
// updates the vault's credit capacity and perform all vault
// state transitions before updating `msg.sender` staked shares
Vault.recalculateVaultsCreditCapacity(vaultsIds);
// get vault staking fee distribution data
@audit>> Distribution.Data storage wethRewardDistribution = vault.wethRewardDistribution;
// cast actor address to bytes32
@audit>> bytes32 actorId = bytes32(uint256(uint160(msg.sender)));
@audit>> // get the claimable amount of fees
@audit>> UD60x18 amountToClaimX18 = vault.wethRewardDistribution.getActorValueChange(actorId).intoUD60x18();
@audit>> // reverts if the claimable amount is NOT 0
@audit>> if (!amountToClaimX18.isZero()) revert Errors.UserHasPendingRewards(actorId, amountToClaimX18.intoUint256()); //@audit NOTE why revert
// accumulate the actor's pending reward before unstaking
@audit>> wethRewardDistribution.accumulateActor(actorId);
Users are not allowed to lose funds in the unstake function but this fees will be lost in the stake function because it lacks the necessary check
/// @notice allows user to claim their share of fees
/// @param vaultId the vault fees are claimed from
function claimFees(uint128 vaultId) external {
// load the vault data storage pointer
Vault.Data storage vault = Vault.load(vaultId);
// get the actor id
bytes32 actorId = bytes32(uint256(uint160(msg.sender)));
// reverts if the actor has no shares
if (vault.wethRewardDistribution.actor[actorId].shares == 0) revert Errors.NoSharesAvailable();
// get the claimable amount of fees
UD60x18 amountToClaimX18 = vault.wethRewardDistribution.getActorValueChange(actorId).intoUD60x18(); // WHEN STAKING MORE WE SHOULD CLAIM FIRST@audit NOTE
// reverts if the claimable amount is 0
if (amountToClaimX18.isZero()) revert Errors.NoFeesToClaim();
vault.wethRewardDistribution.accumulateActor(actorId);
// weth address
address weth = MarketMakingEngineConfiguration.load().weth;
// load the weth collateral data storage pointer
Collateral.Data storage wethCollateral = Collateral.load(weth);
// convert the amount to claim to weth amount
uint256 amountToClaim = wethCollateral.convertUd60x18ToTokenAmount(amountToClaimX18);
// transfer the amount to the claimer
IERC20(weth).safeTransfer(msg.sender, amountToClaim);
// emit event to log the amount claimed
emit LogClaimFees(msg.sender, vaultId, amountToClaim);
}
Impact
Lose of fee distributed to this actor/user.
Tools Used
Manual Review
Recommendations
implement the same check as done in the unstake function
implement another claim function that can allow the contract to call fee-distribution contract directly and forward this rewards to the user.
Updates
Lead Judging Commences
inallhonesty 3 months ago
Submission Judgement Published Assigned finding tags:
Inside VaultRouterBranch if you stake wait some time then stake again makes you lose the rewards.
Prize pool breakdown
Total prize
70,000 USDC
nSLOC
3,25821 USDC / LOC
66,500
3,500
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.