Description

The PeripheryPayments::pullToken function does not perform any access control and can be used to invoke transferFrom on any token.

function pullToken(
    IERC20 token,
    uint256 amount,
    address from,
    address recipient
) public payable {
    token.safeTransferFrom(from, recipient, amount);
}

Furthermore, we have two additional observations about this function:

• It is unnecessarily marked as payable.

• It allows to call transferFrom on any contract, not just ERC20; since ERC721 tokens also have a compatible transferFrom function, pullToken could be used to invoke transferFrom on ERC721 contracts as well. At the time of this review, the Voyage contract does not hold nor is supposed to have approval for any ERC721 assets, so this issue has no impact yet.

Impact

An attacker can use this function to invoke tranferFrom on any contract on behalf of Voyage, with arbitrary arguments. This can be exploited to steal any ERC20 token for which Voyage has received approval.

Recommendations

Apply strict access control to this function, allowing only calls from address(this).

Remediation

Voyage has applied the appropriate level of access control to this function by making it internal. Furthermore, the contract has been removed and its functionality factored into a library as reflected in commit 9a2e8f42↗.