Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Low Risk

sma4

Summary for LOW findings

Number Details Instances
[L-01] Some tokens may revert when zero value transfers are made 3
[L-02] Use Ownable2Step rather than Ownable 1
[L-03] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards 3
[L-04] Contracts do not use their OZ upgradable counterparts 4
[L-05] Constant decimal values 1
[L-06] Missing zero address check in constructor 1
[L-07] Burn functions should be protected with a modifier 2
[L-08] Array does not have a pop function 1

[L‑01] Some tokens may revert when zero value transfers are made

In spite of the fact that EIP-20 states that zero-valued transfers must be accepted, some tokens, such as LEND will revert if this is attempted, which may cause transactions that involve other tokens (such as batch operations) to fully revert. Consider skipping the transfer if the amount is zero, which will also save gas.

​
file: blob/main/src/DSCEngine.sol
​
157 bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
​
274 bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn);
​
287 bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral);
​

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

[L‑02] Use Ownable2Step rather than Ownable

Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

file: main/src/DecentralizedStableCoin.sol
39 contract DecentralizedStableCoin is ERC20Burnable, Ownable {
​

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

[L‑03] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards

Even if the function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks will open the users of this protocol up to read-only reentrancies with no way to protect against it, except by block-listing the whole protocol.

​
file: blob/main/src/DSCEngine.sol
​
157 bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
​
274 bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn);
​
287 bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral);
​

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

[L-04] Contracts do not use their OZ upgradable counterparts

file; main/src/DecentralizedStableCoin.sol
​
26 import {ERC20Burnable, ERC20} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
​
27 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
Contracts do not use their OZ upgradable counterparts

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

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

file: blob/main/src/DSCEngine.sol
​
27 import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
28 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​

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

[L-05] Constant decimal values

The use of fixed decimal values such as 1e18 or 1e8 in Solidity contracts can lead to inaccuracies, bugs, and vulnerabilities, particularly when interacting with tokens having different decimal configurations. Not all ERC20 tokens follow the standard 18 decimal places, and assumptions about decimal places can lead to miscalculations.

Resolution: Always retrieve and use the decimals() function from the token contract itself when performing calculations involving token amounts. This ensures that your contract correctly handles tokens with any number of decimal places, mitigating the risk of numerical errors or under/overflows that could jeopardize contract integrity and user funds.

file: main/src/DSCEngine.sol
​
331 return (collateralAdjustedForThreshold * 1e18) / totalDscMinted;
​

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

[L-06] Missing zero address check in constructor

file: main/src/DSCEngine.sol
​
112 constructor(address[] memory tokenAddresses, address[] memory priceFeedAddresses, address dscAddress) {
​

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

[L-07] Burn functions should be protected with a modifier

file: main/src/DSCEngine.sol
​
212 function burnDsc(uint256 amount) public moreThanZero(amount) {
​
272 function _burnDsc(uint256 amountDscToBurn, address onBehalfOf, address dscFrom) private {
​

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

[L-08] Array does not have a pop function

Array entries are added but are never removed. Consider whether this should be the case, or whether there should be a maximum, or whether old entries should be removed. Cases where there are specific potential problems will be flagged separately under a different issue.

file: main/src/DSCEngine.sol
​
120 s_collateralTokens.push(tokenAddresses[i]);
​

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

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.