--- > **Note:** Aderyn is a constantly evolving, open-source static analysis tool. If your report looks different, is missing detectors or has additional ones included, this is expected. Join the Updraft community on [**GitHub**](https://github.com/Cyfrin/security-and-auditing-full-course-s23/discussions) or [**Discord**](https://discord.gg/cyfrin) if you become stuck or confused. ### Centralization In the last lesson we'd just run Aderyn and were provided with a report.md filled with issues which it detected. Let's assess the first one - it's a biggie. Here's a copy of my Aderyn report for reference: <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) # 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 first vulnerability identified is `## L-1: Centralization Risk for trusted owners`. To be transparent, most of the time this is identified for a protocol, their response will be **_"that's a known issue"_**. Despite this, it's incumbent upon us, as good security researchers, to call this out when we see it. We do this, not least of which, to cover ourselves in the event that the protocol is a rug pull. With that said, it looks like Thunder Loan has some privileged access. We should take a look at the code to identify which powers and functions the owner has. ``` setAllowedToken updateFlashLoanFee _authorizeUpgrade ``` These three functions above have the `onlyOwner` modifier, meaning at any time they can be called by the owner of the protocol. In addition to the above, this protocol utilizes a proxy, so technically the owner could re-write _all_ the functionality of this protocol completely. This is a security concern if this power is held by a single person and should definitely be called out in security reviews. To better visualize the effects of centralization and associated vulnerabilities, check out the [**sc-exploits-minimized**](https://github.com/Cyfrin/sc-exploits-minimized) repo. I've included a [**centralization**](https://github.com/Cyfrin/sc-exploits-minimized/tree/main/src/centralization) section (both a [**contract**](https://github.com/Cyfrin/sc-exploits-minimized/blob/main/src/centralization/Centralization.sol) and [**remix**](https://remix.ethereum.org/#url=https://github.com/Cyfrin/sc-exploits-minimized/blob/main/src/centralization/Centralization.sol&lang=en&optimize=false&runs=200&evmVersion=null&version=soljson-v0.8.20+commit.a1b79de6.js) example) for you to experiment with. In the next lesson, we'll take a close look at the Oasis hack as a case study and see just how impactful `centralization` as a vulnerability can be. See you there!