Cyfrin / 2023-07-foundry-defi-stablecoin

38 stars 33 forks source link

Hardcoded Minimal_Health_Factor Bug in DSCEngine.sol Contract #934

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

Hardcoded Minimal_Health_Factor Bug in DSCEngine.sol Contract

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L74

Summary

The hardcoded Minimal_Health_Factor bug in the DSCEngine.sol contract can cause users to be liquidated even if they have sufficient collateral. This is because the contract hardcodes the Minimal_Health_Factor to 1e18, which is a very high value. This means that even if a user has 200% collateral, they can still be liquidated if the tokens used are USDC or DAI

Vulnerability Details

The contract expect collateral to be 18 decimals but in discord channel it says that it can interact with any chainlink tokens some of them are 6 decimal like USDT or 2 decimals like DAI

Impact

The hardcoded Minimal_Health_Factor bug can have a significant impact on users of the DSCEngine.sol contract. The user can be Immediately liquidated even if they have 200% collateral. This can be a significant financial loss, especially if the user has invested a large amount of money into the contract. Additionally, the liquidation process can be slow and expensive, which can further add to the user's losses.

Tools Used

Manual Review

Recommendations

Use a more sophisticated liquidation mechanism. The current liquidation mechanism is very simplistic and does not take into account the use of different collateral then WETH/WBTC. A more sophisticated liquidation mechanism would take into account the user's collateral and would only liquidate the user if they were truly insolvent.

Darkartt commented 1 year ago

This issue is been put as duplicate of https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/issues/943 which makes it invalid but they are both diffrent issues this is for Hardcoded Health factor while the other one dosnt acount for decimals of token.Both of the issues will presist even if one of them is fixed and shouldnt be treated as the same

PatrickAlphaC commented 1 year ago

The selected report is here:

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/issues/1028

Which states enough details for the client to fix.