The DSCEngine contract contains a potential vulnerability in the _redeemCollateral function,i.e there is no checks to ensure that this line(s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;) is successfully executed in code or not , which can allow users to withdraw collateral tokens without the contract recording it properly. This could lead to a discrepancy in the actual collateral value and the recorded collateral value, potentially resulting in a loss of funds or exploitation of the system.
Vulnerability Details
The issue arises from the fact that the _redeemCollateral function does not check whether the transfer of tokens from the contract to the user (from) was successful. If the transfer function call fails, the contract does not revert, allowing the user to withdraw the collateral tokens without it being properly deducted from their balance in the contract.
This means that a user can exploit this vulnerability by first calling the redeemCollateralForDsc function to burn some DSC, then call the burnDsc function to burn more DSC successfully, and finally call the redeemCollateral function to withdraw collateral tokens without the contract properly recording the deduction.
Impact
The potential impact of this vulnerability is significant. It can lead to an inconsistency in the collateral balance of users and the contract, potentially allowing users to withdraw more collateral tokens than they are entitled to. This could result in a loss of funds for the contract or even enable an attacker to drain collateral tokens from the contract, undermining the stability and integrity of the system.
Tools Used
manual
Recommendations
it is strongly recommended that ensure that this line (s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;) from _redeemCollateral() function, is successfully executed or failed by adding require statement if failed then function must be reverted there. and there is no need to call the transfer function after that.
)
hans-cyfrin
commented
1 year ago
"Collateral Withdrawal Bug in DSCEngine Contract"
Severity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L282-L291
Summary
The DSCEngine contract contains a potential vulnerability in the _redeemCollateral function,i.e there is no checks to ensure that this line(s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;) is successfully executed in code or not , which can allow users to withdraw collateral tokens without the contract recording it properly. This could lead to a discrepancy in the actual collateral value and the recorded collateral value, potentially resulting in a loss of funds or exploitation of the system.
Vulnerability Details
The issue arises from the fact that the _redeemCollateral function does not check whether the transfer of tokens from the contract to the user (from) was successful. If the transfer function call fails, the contract does not revert, allowing the user to withdraw the collateral tokens without it being properly deducted from their balance in the contract.
This means that a user can exploit this vulnerability by first calling the redeemCollateralForDsc function to burn some DSC, then call the burnDsc function to burn more DSC successfully, and finally call the redeemCollateral function to withdraw collateral tokens without the contract properly recording the deduction.
Impact
The potential impact of this vulnerability is significant. It can lead to an inconsistency in the collateral balance of users and the contract, potentially allowing users to withdraw more collateral tokens than they are entitled to. This could result in a loss of funds for the contract or even enable an attacker to drain collateral tokens from the contract, undermining the stability and integrity of the system.
Tools Used
manual
Recommendations
it is strongly recommended that ensure that this line (s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;) from _redeemCollateral() function, is successfully executed or failed by adding require statement if failed then function must be reverted there. and there is no need to call the transfer function after that. )