Skip to content

Commit

Permalink
updated for 0.8.19
Browse files Browse the repository at this point in the history
  • Loading branch information
PatrickAlphaC committed Apr 24, 2024
1 parent 160a8b2 commit bde0564
Show file tree
Hide file tree
Showing 18 changed files with 225 additions and 17 deletions.
208 changes: 208 additions & 0 deletions report.md
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
```



2 changes: 1 addition & 1 deletion script/DeployDSC.s.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;
pragma solidity ^0.8.19;

import { Script } from "forge-std/Script.sol";
import { HelperConfig } from "./HelperConfig.s.sol";
Expand Down
2 changes: 1 addition & 1 deletion script/HelperConfig.s.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.25;
pragma solidity ^0.8.19;

import { MockV3Aggregator } from "../test/mocks/MockV3Aggregator.sol";
import { Script } from "forge-std/Script.sol";
Expand Down
2 changes: 1 addition & 1 deletion src/DSCEngine.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

// SPDX-License-Identifier: MIT

pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { OracleLib, AggregatorV3Interface } from "./libraries/OracleLib.sol";
import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
Expand Down
2 changes: 1 addition & 1 deletion src/DecentralizedStableCoin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
// private
// view & pure functions

pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { ERC20Burnable, ERC20 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/OracleLib.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { AggregatorV3Interface } from "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

Expand Down
2 changes: 1 addition & 1 deletion test/fuzz/continueOnRevert/ContinueOnRevertHandler.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ contract ContinueOnRevertHandler is Test {
/////////////////////////////
// Aggregator //
/////////////////////////////
function updateCollateralPrice(uint128 newPrice, uint256 collateralSeed) public {
function updateCollateralPrice(uint128, /* newPrice */ uint256 collateralSeed) public {
// int256 intNewPrice = int256(uint256(newPrice));
int256 intNewPrice = 0;
ERC20Mock collateral = _getCollateralFromSeed(collateralSeed);
Expand Down
2 changes: 1 addition & 1 deletion test/fuzz/failOnRevert/StopOnRevertHandler.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.25;
pragma solidity ^0.8.19;

import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { Test } from "forge-std/Test.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/fuzz/failOnRevert/StopOnRevertInvariants.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.25;
pragma solidity ^0.8.19;

// Invariants:
// protocol must never be insolvent / undercollateralized
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/ERC20Mock.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

Expand Down
2 changes: 1 addition & 1 deletion test/mocks/MockFailedMintDSC.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { ERC20Burnable, ERC20 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/MockFailedTransfer.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { ERC20Burnable, ERC20 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/MockFailedTransferFrom.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { ERC20Burnable, ERC20 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/MockMoreDebtDSC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
// private
// view & pure functions

pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { ERC20Burnable, ERC20 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/unit/DSCEngineTest.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { DeployDSC } from "../../script/DeployDSC.s.sol";
import { DSCEngine } from "../../src/DSCEngine.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/unit/DecentralizedStablecoinTest.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { DecentralizedStableCoin } from "../../src/DecentralizedStableCoin.sol";
import { Test, console } from "forge-std/Test.sol";
Expand Down
2 changes: 1 addition & 1 deletion test/unit/OracleLibTest.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.8.25;
pragma solidity 0.8.19;

import { MockV3Aggregator } from "../mocks/MockV3Aggregator.sol";
import { Test, console } from "forge-std/Test.sol";
Expand Down

0 comments on commit bde0564

Please sign in to comment.