USDMPegRecovery does not account for fees

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L90-L108
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L110-L128

Vulnerability details

Curve.fi pools charge fees when adding or removing liquidity. The only time fees are not applied are when withdrawals are done using remove_liquidity(). USDMPegRecovery keeps track of tokens deposited and withdrawn, but does not keep track of fees assessed on these operations. The pool in use by this token assesses a 0.040% fee as well as a 50.000% of 0.040% admin fee.

Impact

A whale, or a user/miner using same-block cross-transaction flashloans, can repeatedly deposit and withdraw a large number of tokens, which causes large dollar-value fees to be assessed by the pool, but the number of tokens the user gets is unaffected. The attacker can do this repeatedly to drain all contract funds.

Proof of Concept

These are two separate instances of the issue, as the attacker only needs to exploit one of the functions, then wait for the guardian to call add_liquidity() or remove_liquidity(). The exploits can be applied separately to each of the two tokens.

    function deposit(Liquidity calldata _deposits) external {
        Liquidity memory total = totalLiquidity;
        Liquidity memory user = userLiquidity[msg.sender];
        if(_deposits.usdm > 0) {
            usdm.safeTransferFrom(msg.sender, address(this), uint256(_deposits.usdm));
            total.usdm += _deposits.usdm;
            user.usdm += _deposits.usdm;
        }

        if(_deposits.pool3 > 0) {
            require(totalLiquidity.usdm > 4000000e18, "usdm low");
            pool3.safeTransferFrom(msg.sender, address(this), uint256(_deposits.pool3));
            total.pool3 += _deposits.pool3;
            user.pool3 += _deposits.pool3;
        }
        totalLiquidity = total;
        userLiquidity[msg.sender] = user;
        emit Deposit(msg.sender, _deposits);
    }

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L90-L108

    function withdraw(Liquidity calldata _withdrawal) external {
        Liquidity memory total = totalLiquidity;
        Liquidity memory user = userLiquidity[msg.sender];
        if(_withdrawal.usdm > 0) {
            require(unlockable, "!unlock usdm");
            usdm.safeTransfer(msg.sender, uint256(_withdrawal.usdm));
            total.usdm -= _withdrawal.usdm;
            user.usdm -= _withdrawal.usdm;
        }

        if(_withdrawal.pool3 > 0) {
            pool3.safeTransfer(msg.sender, uint256(_withdrawal.pool3));
            total.pool3 -= _withdrawal.pool3;
            user.pool3 -= _withdrawal.pool3;
        }
        totalLiquidity = total;
        userLiquidity[msg.sender] = user;
        emit Withdraw(msg.sender, _withdrawal);
    }

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L110-L128

Tools Used

Code inspection

Recommended Mitigation Steps

Keep track of fees and reduce deposits/withdrawals accordingly

[r2moon]

duplicated with #188

[GalloDaSballo]

Duplicate of #70