VIS-2 / taobank-04-24

0 stars 0 forks source link

First staker can drain `TBANKStaking` contract #12

Open DanailYordanov opened 4 months ago

DanailYordanov commented 4 months ago

Impact

Severity: High Likelihood: Medium

Context

TBANKStaking::stake()

Description

In the TBANKStaking::stake() function, there is a vulnerability that arises when TBANKStaking::takeFees() is called with an amount of stable coins to be distributed, and the staking contract has no TBANK staked. In this scenario, the F_StableCoin variable is incremented. However, upon a subsequent stake, the totalTBANKStaked is checked in order to give the entire previously accumulated F_StableCoin to the first staker. The issue lies in the fact that stableCoinUserGains[msg.sender] is incremented, allowing a staker to unstake their funds and stake them again to accumulate the same amount repeatedly. This opens the door for malicious users to repeatedly unstake and restake their funds, accumulating a significant value in stableCoinUserGains[msg.sender]. Subsequently, when there are new calls to TBANKStaking::takeFees(), thus enough taoUSD in the contract, the malicious user can drain the entire contract, leaving other users without any accumulated rewards.

function stake(uint256 _tbankAmount) external {
    // ** code **
    // Grab and record accumulated StableCoin gains from the current stake and update Snapshot.
    uint256 currentTotalTBANKStaked = totalTBANKStaked;
    if (currentTotalTBANKStaked == 0)
@>      stableCoinUserGains[msg.sender] += F_StableCoin; // @audit can be exploited by unstaking
    _updateUserSnapshot(msg.sender);
   // ** code **
}

Recommendation

Adjust L109 accordingly:

stableCoinUserGains[msg.sender] += (F_StableCoin - F_StableCoinSnapshots[msg.sender]);

Utilizing this adjustment prevents the accumulation of rewards by restaking and avoids losing already accrued rewards when a staker has unstaked all of their funds and then restaked again.

PoC

To verify the issue, follow this guide and include this test in the codebase. The test demonstrates how a malicious user is able to drain the staking contracts, leaving others with no way to retrieve their funds.