Summary
Event emits can mislead consumers about successful function execution.
When either depositCollateral or redeemCollateral is executed, events are emitted before checking the result returned from the IERC20 interaction.
Vulnerability Details
Although, this vulnerability won't directly lead to stolen funds or malicious actions. They can trick the frontend that the functions in which events are emitted that they are executed without any reverts.
That way new values will be updated incorrectly and can lead to bad UX or even some wrong balance assumptions.
Events, that will be emitted despite the fact functions reverted:
File: src/DSCEngine.sol
event CollateralDeposited(address indexed user, address indexed token, uint256 indexed amount);
event CollateralRedeemed(address indexed redeemedFrom, address indexed redeemedTo, address indexed token, uint256 amount);
Tools Used
Manual
Recommendations
File: src/DSCEngine.sol
function _redeemCollateral(address from, address to, address tokenCollateralAddress, uint256 amountCollateral)
private
{
s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;
- emit CollateralRedeemed(from, to, tokenCollateralAddress, amountCollateral);
bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
+ emit CollateralRedeemed(from, to, tokenCollateralAddress, amountCollateral);
}
File: src/DSCEngine.sol
function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
public
moreThanZero(amountCollateral)
isAllowedToken(tokenCollateralAddress)
nonReentrant
{
s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral;
- emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
+ emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
}
