--- > **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. # 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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L239) ```solidity function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) { ``` - Found in src/protocol/ThunderLoan.sol [Line: 265](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L265) ```solidity function updateFlashLoanFee(uint256 newFee) external onlyOwner { ``` - Found in src/protocol/ThunderLoan.sol [Line: 292](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L292) ```solidity function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 238](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L238) ```solidity function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 264](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L264) ```solidity function updateFlashLoanFee(uint256 newFee) external onlyOwner { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 287](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L231) ```solidity function repay(IERC20 token, uint256 amount) public { ``` - Found in src/protocol/ThunderLoan.sol [Line: 276](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L276) ```solidity function getAssetFromToken(IERC20 token) public view returns (AssetToken) { ``` - Found in src/protocol/ThunderLoan.sol [Line: 280](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L280) ```solidity function isCurrentlyFlashLoaning(IERC20 token) public view returns (bool) { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 230](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L230) ```solidity function repay(IERC20 token, uint256 amount) public { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 275](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L275) ```solidity function getAssetFromToken(IERC20 token) public view returns (AssetToken) { ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 279](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/AssetToken.sol#L31) ```solidity event ExchangeRateUpdated(uint256 newExchangeRate); ``` - Found in src/protocol/ThunderLoan.sol [Line: 105](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L105) ```solidity event Deposit(address indexed account, IERC20 indexed token, uint256 amount); ``` - Found in src/protocol/ThunderLoan.sol [Line: 106](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L106) ```solidity event AllowedTokenSet(IERC20 indexed token, AssetToken indexed asset, bool allowed); ``` - Found in src/protocol/ThunderLoan.sol [Line: 107](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L107) ```solidity event Redeemed( ``` - Found in src/protocol/ThunderLoan.sol [Line: 110](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L105) ```solidity event Deposit(address indexed account, IERC20 indexed token, uint256 amount); ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 106](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L106) ```solidity event AllowedTokenSet(IERC20 indexed token, AssetToken indexed asset, bool allowed); ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 107](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/upgradedProtocol/ThunderLoanUpgraded.sol#L107) ```solidity event Redeemed( ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 110](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/interfaces/IFlashLoanReceiver.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/interfaces/IPoolFactory.sol [Line: 2](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/interfaces/IPoolFactory.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/interfaces/ITSwapPool.sol [Line: 2](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/interfaces/ITSwapPool.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/interfaces/IThunderLoan.sol [Line: 2](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/interfaces/IThunderLoan.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/protocol/AssetToken.sol [Line: 2](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/AssetToken.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/protocol/OracleUpgradeable.sol [Line: 2](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/OracleUpgradeable.sol#L2) ```solidity pragma solidity 0.8.20; ``` - Found in src/protocol/ThunderLoan.sol [Line: 64](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L64) ```solidity pragma solidity 0.8.20; ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 64](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/src/protocol/ThunderLoan.sol#L292) ```solidity function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } ``` - Found in src/upgradedProtocol/ThunderLoanUpgraded.sol [Line: 287](https://github.com/Cyfrin/6-thunder-loan-audit/blob/2250d81b89aebdd9cb135382e068af8c269e3a4b/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!
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 or Discord if you become stuck or confused.
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:
This report was generated by Aderyn, a static analysis tool built by Cyfrin, 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.
Key | Value |
---|---|
.sol Files | 8 |
Total nSLOC | 461 |
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 |
Category | No. of Issues |
---|---|
High | 0 |
Low | 6 |
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
Found in src/protocol/ThunderLoan.sol Line: 265
Found in src/protocol/ThunderLoan.sol Line: 292
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 238
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 264
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 287
address(0)
when assigning values to address state variablesCheck for address(0)
when assigning values to address state variables.
Found in src/protocol/OracleUpgradeable.sol Line: 16
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
Found in src/protocol/ThunderLoan.sol Line: 276
Found in src/protocol/ThunderLoan.sol Line: 280
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 230
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 275
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 279
indexed
fieldsIndex 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
Found in src/protocol/ThunderLoan.sol Line: 105
Found in src/protocol/ThunderLoan.sol Line: 106
Found in src/protocol/ThunderLoan.sol Line: 107
Found in src/protocol/ThunderLoan.sol Line: 110
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 105
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 106
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 107
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 110
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
Found in src/interfaces/IPoolFactory.sol Line: 2
Found in src/interfaces/ITSwapPool.sol Line: 2
Found in src/interfaces/IThunderLoan.sol Line: 2
Found in src/protocol/AssetToken.sol Line: 2
Found in src/protocol/OracleUpgradeable.sol Line: 2
Found in src/protocol/ThunderLoan.sol Line: 64
Found in src/upgradedProtocol/ThunderLoanUpgraded.sol Line: 64
Consider removing empty blocks.
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.
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 repo. I've included a centralization section (both a contract and remix 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!
Learn the impact of centralization and discuss the importance of reporting such risks in private audits. Highlights case studies like Oasis.
Previous lesson
Previous
Next lesson
Next
Give us feedback
Duration: 25min
Duration: 1h 18min
Duration: 35min
Duration: 2h 28min
Duration: 5h 03min
Duration: 5h 22min
Duration: 4h 33min
Duration: 2h 01min
Duration: 1h 40min