Description

The PeripheryPayments::approve function does not perform any access control and can be used to invoke approve on any token on behalf of the Voyage contract.

function approve(
    IERC20 token,
    address to,
    uint256 amount
) public payable {
    token.safeApprove(to, amount);
}

Furthermore, we have two additional observations about this function:

• It is unnecessarily marked as payable.

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

Impact

An attacker can use this function to invoke approve on any contract on behalf of Voyage, with arbitrary arguments. This can be exploited to gain approval for any ERC20 or ERC721 token owned by Voyage. At the time of this review, the main Voyage contract only temporarily holds assets (e.g., while processing buyNow), so this could only be exploited if an external call to a malicious contract was to be performed while Voyage is in possession of an asset.

While this issue might not be exploitable in the code as reviewed, we strongly recommend against exposing this function, as approval for a token has a persistent effect that might become relevant with a future code update.

Recommendations

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

Remediation

Commit 9a2e8f42↗ was indicated as containing the remediation. The commit correctly fixes the issue by moving the approve function a new LibPayments library as an internal function. The subsequent commit 9a2e8f42↗ removes the PeripheryPayments.sol file entirely, leaving only the library version of the function.