Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/modules/protoforms/BaseVault.sol#L58-L117

Vulnerability details

Impact

These three transfer functions allow an attacker to supply an arbitrary from and to to transfer ERC20s, ERC721s, and ERC1155s. The moment that a user sets approval for the contract to spend their tokens, an attacker can front-run the next call and steal the tokens for themselves.

Proof of Concept

This function allows anyone to transfer anyone's ERC20s given that the owner has already provided approval to the contract.

    function batchDepositERC20(
        address _from,
        address _to,
        address[] calldata _tokens,
        uint256[] calldata _amounts
    ) external {
        for (uint256 i = 0; i < _tokens.length; ) {
            IERC20(_tokens[i]).transferFrom(_from, _to, _amounts[i]);
            unchecked {
                ++i;
            }
        }
    }

The same is true for the ERC721 and ERC1155 deposit functions.

Tools Used

Manual review.

Recommended Mitigation Steps

Do not allow users to supply arbitrary to and from parameters to the token transfer functions.

code-423n4 / 2022-07-fractional-findings