--- ### Static Analysis Continued _We'll be referencing our Aderyn's report.md output for this lesson, your specific output may differ from mine._ <details> <summary>report.md</summary> # 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 - [Aderyn Analysis Report](#aderyn-analysis-report) - [Table of Contents](#table-of-contents) - [Summary](#summary) - [Files Summary](#files-summary) - [Files Details](#files-details) - [Issue Summary](#issue-summary) - [Low Issues](#low-issues) - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) - [L-2: Missing checks for `address(0)` when assigning values to address state variables](#l-2-missing-checks-for-address0-when-assigning-values-to-address-state-variables) - [L-3: `public` functions not used internally could be marked `external`](#l-3-public-functions-not-used-internally-could-be-marked-external) - [L-4: Event is missing `indexed` fields](#l-4-event-is-missing-indexed-fields) - [L-5: PUSH0 is not supported by all chains](#l-5-push0-is-not-supported-by-all-chains) - [L-6: Empty Block](#l-6-empty-block) - [Wrap Up](#wrap-up) # Summary ## Files Summary | Key | Value | | ----------- | ----- | | .sol Files | 8 | | Total nSLOC | 461 | ## Files Details | Filepath | nSLOC | | -------------------------------------------- | ------- | | src/interfaces/IFlashLoanReceiver.sol | 13 | | src/interfaces/IPoolFactory.sol | 4 | | src/interfaces/ITSwapPool.sol | 4 | | src/interfaces/IThunderLoan.sol | 4 | | src/protocol/AssetToken.sol | 65 | | src/protocol/OracleUpgradeable.sol | 23 | | src/protocol/ThunderLoan.sol | 176 | | src/upgradedProtocol/ThunderLoanUpgraded.sol | 172 | | **Total** | **461** | ## Issue Summary | Category | No. of Issues | | -------- | ------------- | | High | 0 | | Low | 6 | # 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/protocol/ThunderLoan.sol [Line: 239](src/protocol/ThunderLoan.sol#L239) ```solidity function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) { ``` - Found in src/protocol/ThunderLoan.sol [Line: 265](src/protocol/ThunderLoan.sol#L265) ```solidity function updateFlashLoanFee(uint256 newFee) external onlyOwner { ``` - Found in src/protocol/ThunderLoan.sol [Line: 292](src/protocol/ThunderLoan.sol#L292) ```solidity function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 238](src/upgradedProtocol/ThunderLoanUpgraded.sol#L238) ```solidity function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 264](src/upgradedProtocol/ThunderLoanUpgraded.sol#L264) ```solidity function updateFlashLoanFee(uint256 newFee) external onlyOwner { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 287](src/upgradedProtocol/ThunderLoanUpgraded.sol#L287) ```solidity function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } ``` ## L-2: 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/protocol/OracleUpgradeable.sol [Line: 16](src/protocol/OracleUpgradeable.sol#L16) ```solidity s_poolFactory = poolFactoryAddress; ``` ## L-3: `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/protocol/ThunderLoan.sol [Line: 231](src/protocol/ThunderLoan.sol#L231) ```solidity function repay(IERC20 token, uint256 amount) public { ``` - Found in src/protocol/ThunderLoan.sol [Line: 276](src/protocol/ThunderLoan.sol#L276) ```solidity function getAssetFromToken(IERC20 token) public view returns (AssetToken) { ``` - Found in src/protocol/ThunderLoan.sol [Line: 280](src/protocol/ThunderLoan.sol#L280) ```solidity function isCurrentlyFlashLoaning(IERC20 token) public view returns (bool) { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 230](src/upgradedProtocol/ThunderLoanUpgraded.sol#L230) ```solidity function repay(IERC20 token, uint256 amount) public { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 275](src/upgradedProtocol/ThunderLoanUpgraded.sol#L275) ```solidity function getAssetFromToken(IERC20 token) public view returns (AssetToken) { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 279](src/upgradedProtocol/ThunderLoanUpgraded.sol#L279) ```solidity function isCurrentlyFlashLoaning(IERC20 token) public view returns (bool) { ``` ## L-4: 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/protocol/AssetToken.sol [Line: 31](src/protocol/AssetToken.sol#L31) ```solidity event ExchangeRateUpdated(uint256 newExchangeRate); ``` - Found in src/protocol/ThunderLoan.sol [Line: 105](src/protocol/ThunderLoan.sol#L105) ```solidity event Deposit(address indexed account, IERC20 indexed token, uint256 amount); ``` - Found in src/protocol/ThunderLoan.sol [Line: 106](src/protocol/ThunderLoan.sol#L106) ```solidity event AllowedTokenSet(IERC20 indexed token, AssetToken indexed asset, bool allowed); ``` - Found in src/protocol/ThunderLoan.sol [Line: 107](src/protocol/ThunderLoan.sol#L107) ```solidity event Redeemed( ``` - Found in src/protocol/ThunderLoan.sol [Line: 110](src/protocol/ThunderLoan.sol#L110) ```solidity event FlashLoan(address indexed receiverAddress, IERC20 indexed token, uint256 amount, uint256 fee, bytes params); ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 105](src/upgradedProtocol/ThunderLoanUpgraded.sol#L105) ```solidity event Deposit(address indexed account, IERC20 indexed token, uint256 amount); ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 106](src/upgradedProtocol/ThunderLoanUpgraded.sol#L106) ```solidity event AllowedTokenSet(IERC20 indexed token, AssetToken indexed asset, bool allowed); ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 107](src/upgradedProtocol/ThunderLoanUpgraded.sol#L107) ```solidity event Redeemed( ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 110](src/upgradedProtocol/ThunderLoanUpgraded.sol#L110) ```solidity event FlashLoan(address indexed receiverAddress, IERC20 indexed token, uint256 amount, uint256 fee, bytes params); ``` ## L-5: PUSH0 is not supported by all chains Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail. - Found in src/interfaces/IFlashLoanReceiver.sol [Line: 2](src/interfaces/IFlashLoanReceiver.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/interfaces/IPoolFactory.sol [Line: 2](src/interfaces/IPoolFactory.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/interfaces/ITSwapPool.sol [Line: 2](src/interfaces/ITSwapPool.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/interfaces/IThunderLoan.sol [Line: 2](src/interfaces/IThunderLoan.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/protocol/AssetToken.sol [Line: 2](src/protocol/AssetToken.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/protocol/OracleUpgradeable.sol [Line: 2](src/protocol/OracleUpgradeable.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/protocol/ThunderLoan.sol [Line: 64](src/protocol/ThunderLoan.sol#L64) ```solidity pragma solidity 0.8.20; ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 64](src/upgradedProtocol/ThunderLoanUpgraded.sol#L64) ```solidity pragma solidity 0.8.20; ``` ## L-6: Empty Block Consider removing empty blocks. - Found in src/protocol/ThunderLoan.sol [Line: 292](src/protocol/ThunderLoan.sol#L292) ```solidity function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 287](src/upgradedProtocol/ThunderLoanUpgraded.sol#L287) ```solidity function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } ``` </details> The remainder of detections by `Aderyn` are going to qualify as gas/informational findings. As such we'll likely just copy these outputs into our audit report - this is a huge time saving benefit of `Aderyn`. Great first finding by `Aderyn`! What was pointed out next? ``` ## L-2: Missing checks for `address(0)` when assigning values to address state variables - Found in src/protocol/OracleUpgradeable.sol [Line: 16](src/protocol/OracleUpgradeable.sol#L16) ``` We've seen this one before! This is another great heads up to give the protocol. Navigate to the contract/line identified by the detector and let's add a note. ```js function __Oracle_init_unchained(address poolFactoryAddress) internal onlyInitializing { // @Audit-Informational: Written in Aderyn s_poolFactory = poolFactoryAddress; } ``` Do the same for the remainder of the findings identified in `Aderyn`'s report. We'll come back to these when we need to add them to our final write up! ``` ## L-3: `public` functions not used internally could be marked `external` ## L-4: Event is missing `indexed` fields ## L-5: PUSH0 is not supported by all chains ## L-6: Empty Block ``` ### Wrap Up Whew, Slither and Aderyn have been very helpful in identifying some of these vulnerabilities already. Pointing out vulnerabilities like centralization can at times be contentious, but it's an important part of our role as security researchers. Circumstances like the Oasis case study show that risks like these, while accepted by many protocols should be clearly communicated to people. That's where we come in. Let's look at iPoolFactory.sol in the next lesson!