Allowance.sol:: function 'safeApproveFallbackToMax' subject to reentrancy

[c4-bot-3]

Lines of code

https://github.com/reserve-protocol/protocol/blob/master/contracts/libraries/Allowance.sol#L17-L46

Vulnerability details

Impact

The function safeApproveFallbackToMax in Allowance.sol is susceptible to a potential re-entrancy attack.

This occurs in this context due to the external calls to the approve function of the IERC20ApproveOnly interface.

A re-entrancy attack is a type of vulnerability in which an attacker can repeatedly call a function and manipulate its state before the function finishes executing. This situation occurs primarily due to the execution of external contract calls within the function.

Proof of Concept

https://github.com/reserve-protocol/protocol/blob/master/contracts/libraries/Allowance.sol#L17-L46

// 1. Set initial allowance to 0
token.approve(spender, 0);

// some code

// 2. Try to set the provided allowance
try token.approve(spender, value) {...}

// some code

// 3. Fall-back to setting a maximum allowance
if (!success) { token.approve(spender, type(uint256).max); ...}

These token.approve calls are external and could potentially lead to re-entrant calls if the spender contract's fallback function is malicious and calls back into the contract.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate against re-entrancy attacks, the Check-Effects-Interactions pattern should be implemented.

1. First, check if the conditions are fulfilled.
2. Secondly, change the states or effects.
3. Lastly, interact with other contracts.

Ideally, external function calls should be at the end of the execution of a contract's function, and all state variables should be updated before making the call.

Further, using the re-entrancy guard modifier (like nonReentrant modifier available in OpenZeppelin library) can also provide an additional layer of security against re-entrancy attacks.

Assessed type

Reentrancy