code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/vault/VaultReserve.sol#L43-L62

Vulnerability details

Impact

ETH can be transfered to the contract without being computed as a deposit.

Proof of Concept

The function deposit#VaultReserve.sol can accept both tokens and ETH.
Suppose that Vault accidentally transfer eth and an amount of tokens . The contract will deposit the tokens and compute them, but ETH will be received and the deposit will not be computed.

Tools Used

Manual code review

Recommended Mitigation Steps

It is better to be sure that msg.value==0 when token!= address(0). In addittion I would put the code in an if / else clause to be sure that an event will be always emitted ( Now when you deposit ETH you have no event)

function deposit(address token, uint256 amount)
    external
    payable
    override
    onlyVault
    returns (bool)
{
    if (token == address(0)) {
        require(msg.value == amount, Error.INVALID_AMOUNT);
        _balances[msg.sender][token] += msg.value;

}
    else{
        require(msg.value == 0);     @audit  this new line avoid uncomputed eth
        uint256 balance = IERC20(token).balanceOf(address(this));
        IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
        uint256 newBalance = IERC20(token).balanceOf(address(this));
        uint256 received = newBalance - balance;
        require(received >= amount, Error.INVALID_AMOUNT);
        _balances[msg.sender][token] += received;

}
emit Deposit(msg.sender, token, amount);
return true;
}

gzeoneth commented 2 years ago

Invalid because onlyVault.

chase-manning commented 2 years ago

https://github.com/backdfund/protocol/pull/285

code-423n4 / 2022-04-backd-findings