-
Notifications
You must be signed in to change notification settings - Fork 124
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* update version of cyfrin/foundry-devops to 0.1.0 * update solidity version of contracts to 0.8.25 * adding in-line config to continueOnRevertInvariants: `/// forge-config: default.invariant.fail-on-revert = false` * updating solidity version of ERC20Mock * fixed small error in stopOnRevertHandler * updated for 0.8.19 --------- Co-authored-by: Fabriziogianni7 <[email protected]>
- Loading branch information
1 parent
cf9143a
commit 68fcbda
Showing
5 changed files
with
383 additions
and
201 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ clean :; forge clean | |
# Remove modules | ||
remove :; rm -rf .gitmodules && rm -rf .git/modules/* && rm -rf lib && touch .gitmodules && git add . && git commit -m "modules" | ||
|
||
install :; forge install cyfrin/foundry-devops@0.0.11 --no-commit && forge install smartcontractkit/[email protected] --no-commit && forge install foundry-rs/[email protected] --no-commit && forge install openzeppelin/[email protected] --no-commit | ||
install :; forge install cyfrin/foundry-devops@0.1.0 --no-commit && forge install smartcontractkit/[email protected] --no-commit && forge install foundry-rs/[email protected] --no-commit && forge install openzeppelin/[email protected] --no-commit | ||
|
||
# Update Dependencies | ||
update:; forge update | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,208 @@ | ||
# Aderyn Analysis Report | ||
|
||
This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a static analysis tool built by [Cyfrin](https://cyfrin.io), a blockchain security company. This report is not a substitute for manual audit or security review. It should not be relied upon for any purpose other than to assist in the identification of potential security vulnerabilities. | ||
# Table of Contents | ||
|
||
- [Summary](#summary) | ||
- [Files Summary](#files-summary) | ||
- [Files Details](#files-details) | ||
- [Issue Summary](#issue-summary) | ||
- [High Issues](#high-issues) | ||
- [H-1: Arbitrary `from` passed to `transferFrom` (or `safeTransferFrom`)](#h-1-arbitrary-from-passed-to-transferfrom-or-safetransferfrom) | ||
- [Low Issues](#low-issues) | ||
- [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) | ||
- [L-2: Unsafe ERC20 Operations should not be used](#l-2-unsafe-erc20-operations-should-not-be-used) | ||
- [L-3: Missing checks for `address(0)` when assigning values to address state variables](#l-3-missing-checks-for-address0-when-assigning-values-to-address-state-variables) | ||
- [L-4: `public` functions not used internally could be marked `external`](#l-4-public-functions-not-used-internally-could-be-marked-external) | ||
- [L-5: Event is missing `indexed` fields](#l-5-event-is-missing-indexed-fields) | ||
- [L-6: The `nonReentrant` `modifier` should occur before all other modifiers](#l-6-the-nonreentrant-modifier-should-occur-before-all-other-modifiers) | ||
|
||
|
||
# Summary | ||
|
||
## Files Summary | ||
|
||
| Key | Value | | ||
| --- | --- | | ||
| .sol Files | 3 | | ||
| Total nSLOC | 321 | | ||
|
||
|
||
## Files Details | ||
|
||
| Filepath | nSLOC | | ||
| --- | --- | | ||
| src/DSCEngine.sol | 269 | | ||
| src/DecentralizedStableCoin.sol | 29 | | ||
| src/libraries/OracleLib.sol | 23 | | ||
| **Total** | **321** | | ||
|
||
|
||
## Issue Summary | ||
|
||
| Category | No. of Issues | | ||
| --- | --- | | ||
| High | 1 | | ||
| Low | 6 | | ||
|
||
|
||
# High Issues | ||
|
||
## H-1: Arbitrary `from` passed to `transferFrom` (or `safeTransferFrom`) | ||
|
||
Passing an arbitrary `from` address to `transferFrom` (or `safeTransferFrom`) can lead to loss of funds, because anyone can transfer tokens from the `from` address if an approval is made. | ||
|
||
- Found in src/DSCEngine.sol [Line: 308](src/DSCEngine.sol#L308) | ||
|
||
```solidity | ||
bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn); | ||
``` | ||
|
||
|
||
|
||
# Low Issues | ||
|
||
## L-1: Centralization Risk for trusted owners | ||
|
||
Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds. | ||
|
||
- Found in src/DecentralizedStableCoin.sol [Line: 42](src/DecentralizedStableCoin.sol#L42) | ||
|
||
```solidity | ||
contract DecentralizedStableCoin is ERC20Burnable, Ownable { | ||
``` | ||
|
||
- Found in src/DecentralizedStableCoin.sol [Line: 57](src/DecentralizedStableCoin.sol#L57) | ||
|
||
```solidity | ||
function burn(uint256 _amount) public override onlyOwner { | ||
``` | ||
|
||
- Found in src/DecentralizedStableCoin.sol [Line: 68](src/DecentralizedStableCoin.sol#L68) | ||
|
||
```solidity | ||
function mint(address _to, uint256 _amount) external onlyOwner returns (bool) { | ||
``` | ||
|
||
|
||
|
||
## L-2: Unsafe ERC20 Operations should not be used | ||
|
||
ERC20 functions may not behave as expected. For example: return values are not always meaningful. It is recommended to use OpenZeppelin's SafeERC20 library. | ||
|
||
- Found in src/DSCEngine.sol [Line: 280](src/DSCEngine.sol#L280) | ||
|
||
```solidity | ||
bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral); | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 299](src/DSCEngine.sol#L299) | ||
|
||
```solidity | ||
bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral); | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 308](src/DSCEngine.sol#L308) | ||
|
||
```solidity | ||
bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn); | ||
``` | ||
|
||
|
||
|
||
## L-3: Missing checks for `address(0)` when assigning values to address state variables | ||
|
||
Check for `address(0)` when assigning values to address state variables. | ||
|
||
- Found in src/DSCEngine.sol [Line: 256](src/DSCEngine.sol#L256) | ||
|
||
```solidity | ||
s_DSCMinted[msg.sender] += amountDscToMint; | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 278](src/DSCEngine.sol#L278) | ||
|
||
```solidity | ||
s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral; | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 297](src/DSCEngine.sol#L297) | ||
|
||
```solidity | ||
s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral; | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 306](src/DSCEngine.sol#L306) | ||
|
||
```solidity | ||
s_DSCMinted[onBehalfOf] -= amountDscToBurn; | ||
``` | ||
|
||
|
||
|
||
## L-4: `public` functions not used internally could be marked `external` | ||
|
||
Instead of marking a function as `public`, consider marking it as `external` if it is not used internally. | ||
|
||
- Found in src/DecentralizedStableCoin.sol [Line: 57](src/DecentralizedStableCoin.sol#L57) | ||
|
||
```solidity | ||
function burn(uint256 _amount) public override onlyOwner { | ||
``` | ||
|
||
- Found in src/libraries/OracleLib.sol [Line: 20](src/libraries/OracleLib.sol#L20) | ||
|
||
```solidity | ||
function staleCheckLatestRoundData(AggregatorV3Interface chainlinkFeed) | ||
``` | ||
|
||
- Found in src/libraries/OracleLib.sol [Line: 37](src/libraries/OracleLib.sol#L37) | ||
|
||
```solidity | ||
function getTimeout(AggregatorV3Interface /* chainlinkFeed */ ) public pure returns (uint256) { | ||
``` | ||
|
||
|
||
|
||
## L-5: Event is missing `indexed` fields | ||
|
||
Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed. | ||
|
||
- Found in src/DSCEngine.sol [Line: 95](src/DSCEngine.sol#L95) | ||
|
||
```solidity | ||
event CollateralRedeemed(address indexed redeemFrom, address indexed redeemTo, address token, uint256 amount); // if | ||
``` | ||
|
||
|
||
|
||
## L-6: The `nonReentrant` `modifier` should occur before all other modifiers | ||
|
||
This is a best-practice to protect against reentrancy in other modifiers. | ||
|
||
- Found in src/DSCEngine.sol [Line: 183](src/DSCEngine.sol#L183) | ||
|
||
```solidity | ||
nonReentrant | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 222](src/DSCEngine.sol#L222) | ||
|
||
```solidity | ||
nonReentrant | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 255](src/DSCEngine.sol#L255) | ||
|
||
```solidity | ||
function mintDsc(uint256 amountDscToMint) public moreThanZero(amountDscToMint) nonReentrant { | ||
``` | ||
|
||
- Found in src/DSCEngine.sol [Line: 275](src/DSCEngine.sol#L275) | ||
|
||
```solidity | ||
nonReentrant | ||
``` | ||
|
||
|
||
|
Oops, something went wrong.