Assessment reportsPublic findings
Back to Zellic site
↗
Assessment reports>Avantis>Critical findings>Locked shares have undue access to historical rewards
GeneralOverview
Findings
Critical (6)Stop loss higher than `openPrice` causes fund lossUnsafe cast in take profit can lead to fund lossNo access control on `setWithdrawThreshold`Reserve requirement checked before withdrawalLocked shares have undue access to rewardsMax profit can exceed amount reserved from vault
High (9)
Medium (6)
Low (10)
Informational (7)
DiscussionReferral code incentives are misalignedVeTranche NFT info should be structNo way to remove user from Trading whitelistPyth price can become negative or erraticTranche has unnecessary `ReentrancyGuard`Checks-effects-interactions pattern brokenTypos
Threat ModelWhat are threat models?Execute.solReferral.solTrading.solTradingStorage.solTranche.solVaultManager.solVeTranche.sol
Audit ResultsSummary
Category: Coding Mistakes

Locked shares have undue access to historical rewards

Critical Severity
Critical Impact
High Likelihood

Description

In VeTranche, the rewardsDistributedPerSharePerLockPoint state variable increases in order to give rewards to all of the current locked positions. Each time rewards are distributed at the request of the owner of the lock or when the lock expires, the difference between the current value of this state variable and the last checkpoint for this variable is awarded:

function _updateReward(uint256 _id) internal {
    if(lastSharePoint[_id] == rewardsDistributedPerSharePerLockPoint)
        return;

    uint256 pendingReward =
        ((rewardsDistributedPerSharePerLockPoint - lastSharePoint[_id]) *
        tokensByTokenId[_id] * 
        lockMultiplierByTokenId[_id]) /
        (_PRECISION **3);
    rewardsByTokenId[_id] += pendingReward;
    lastSharePoint[_id] = rewardsDistributedPerSharePerLockPoint;
}

However, when a locked position is initially created, lastSharePoint[id] is not set to the current value of rewardsDistributedPerSharePerLockPoint, and it is uninitialized and zero:

function lock(uint256 shares, uint endTime)
public nonReentrant returns (uint256) {
    //...
    tokensByTokenId[nextTokenId] = shares;
    lockTimeByTokenId[nextTokenId] = endTime;
    lockStartTimeByTokenId[nextTokenId] = block.timestamp;
    rewardsByTokenId[nextTokenId] = 0;
    lockMultiplierByTokenId[nextTokenId] =
        getLockPoints(endTime - block.timestamp);
    totalLockPoints +=
        (shares * lockMultiplierByTokenId[nextTokenId]) / _PRECISION;

Impact

A newly locked position will be incorrectly awarded a share of all the rewards distributed before it was locked. This causes the VeTranche to immediately become insolvent because the expected amount of USDC it holds is more than the amount it will actually hold. The impact is that later reward claimants will not be able to claim rewards.

Here is the output from the POC:

Start
LP 1 locks
LP 2 locks
 - rewardsDistributedPerSharePerLockPoint = 0
 - LP 1 reward = 0
 - LP 2 reward = 0
 - LP 3 reward = 0
Warp 7 days, rewards distributed
LP 1 unlocks
 - rewardsDistributedPerSharePerLockPoint = 811376886
 - LP 1 reward = 28815579507696
 - LP 2 reward = 0
 - LP 3 reward = 0
LP 3 locks and immediately unlocks
 - rewardsDistributedPerSharePerLockPoint = 811376886
 - LP 1 reward = 28815579507696
 - LP 2 reward = 0
 - LP 3 reward = 12738897024053
LP 2 tries to unlock
[FAIL. Reason: ERC20: transfer amount exceeds balance]

After rewards are distributed, LP 3 locks and then immediately unlocks, which means they should not get any rewards. However, it does get rewards, and then after this, LP 2 cannot unlock due to insufficient funds in the VeTranche.

Recommendations

Set the lastSharePoint[id] checkpoint to the current value of rewardsDistributedPerSharePerLockPoint in the lock function.

Remediation

This issue has been acknowledged by Avantis Labs, Inc., and a fix was implemented in commit fc4d4be1↗.

Zellic © 2025Back to top ↑