code-423n4 / 2023-10-ens-findings

8 stars 6 forks source link

Lack of Persistent Authorization Verification for Proxy Delegator Token Transfers #56

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-ens/blob/main/contracts/ERC20MultiDelegate.sol#L16-L19 https://github.com/code-423n4/2023-10-ens/blob/main/contracts/ERC20MultiDelegate.sol#L163-L171

Vulnerability details

Impact

The system inherently trusts that each ERC20ProxyDelegator contract will retain its initial authorization for token transfers to the ERC20MultiDelegate contract. However, if this initial authorization is ever altered or removed, the system could face unexpected failures in token transfers. Such a disruption could be exploited by a malicious actor who can interact directly with the proxy contract's ERC20 methods, leading to potential interruptions in the delegation process.

Proof of Concept

constructor function and transferBetweenDelegators function.

constructor(ERC20Votes _token, address _delegate) {
    _token.approve(msg.sender, type(uint256).max);
    _token.delegate(_delegate);
...
...
...
function transferBetweenDelegators(
    address from,
    address to,
    uint256 amount
) internal {
    address proxyAddressFrom = retrieveProxyContractAddress(token, from);
    address proxyAddressTo = retrieveProxyContractAddress(token, to);
    token.transferFrom(proxyAddressFrom, proxyAddressTo, amount);
}

Though the proxy contract sets an initial approval for the ERC20MultiDelegate upon creation, the system doesn't check this approval's persistence during subsequent transfers.

Recommended Mitigation Steps

Introduce checks before token movements to confirm that the proxy contract still maintains its approval for the ERC20MultiDelegate. Implement safeguard mechanisms to ensure that the initial permissions granted to proxy contracts are irreversible or at least shielded from external manipulations. Regularly monitor the proxy contracts' approval states, and if any anomalies are detected, corrective actions should be taken immediately.

Assessed type

Invalid Validation

c4-pre-sort commented 11 months ago

141345 marked the issue as low quality report

141345 commented 11 months ago

invalid

maximum approval, no other way to change

c4-judge commented 10 months ago

hansfriese marked the issue as unsatisfactory: Invalid