FlasherFTM - Unsolicited invocation of the callback (CREAM auth bypass)
TL;DR: Anyone can call `ICTokenFlashloan(crToken).flashLoan(address(FlasherFTM), address(FlasherFTM), info.amount, params)` directly and pass validation checks in `onFlashLoan()`. This call forces it to accept unsolicited flash loans and execute the actions provided under the attacker's `FlashLoan.Info`.\\n`receiver.onFlashLoan(initiator, token, amount, ...)` is called when receiving a flash loan. According to EIP-3156, the `initiator` is `msg.sender` so that one can use it to check if the call to `receiver.onFlashLoan()` was unsolicited or not.\\nThird-party Flash Loan provider contracts are often upgradeable.\\nFor example, the Geist lending contract configured with this system is upgradeable. Upgradeable contracts bear the risk that one cannot assume that the contract is always running the same code. In the worst case, for example, a malicious proxy admin (leaked keys, insider, …) could upgrade the contract and perform unsolicited calls with arbitrary data to Flash Loan consumers in an attempt to exploit them. It, therefore, is highly recommended to verify that flash loan callbacks in the system can only be called if the contract was calling out to the provider to provide a Flash Loan and that the conditions of the flash loan (returned data, amount) are correct.\\nNot all Flash Loan providers implement EIP-3156 correctly.\\nCream Finance, for example, allows users to set an arbitrary `initiator` when requesting a flash loan. This deviates from EIP-3156 and was reported to the Cream development team as a security issue. Hence, anyone can spoof that `initiator` and potentially bypass authentication checks in the consumers' `receiver.onFlashLoan()`. Depending on the third-party application consuming the flash loan is doing with the funds, the impact might range from medium to critical with funds at risk. For example, projects might assume that the flash loan always originates from their trusted components, e.g., because they use them to refinance switching funds between pools or protocols.\\nThe `FlasherFTM` contract assumes that flash loans for the Flasher can only be initiated by authorized callers (isAuthorized) - for a reason - because it is vital that the `FlashLoan.Info calldata info` parameter only contains trusted data:\\n```\\n/\\*\\*\\n \\* @dev Routing Function for Flashloan Provider\\n \\* @param info: struct information for flashLoan\\n \\* @param \\_flashnum: integer identifier of flashloan provider\\n \\*/\\nfunction initiateFlashloan(FlashLoan.Info calldata info, uint8 \\_flashnum) external isAuthorized override {\\n if (\\_flashnum == 0) {\\n \\_initiateGeistFlashLoan(info);\\n } else if (\\_flashnum == 2) {\\n \\_initiateCreamFlashLoan(info);\\n } else {\\n revert(Errors.VL\\_INVALID\\_FLASH\\_NUMBER);\\n }\\n}\\n```\\n\\n```\\nmodifier isAuthorized() {\\n require(\\n msg.sender == \\_fujiAdmin.getController() ||\\n msg.sender == \\_fujiAdmin.getFliquidator() ||\\n msg.sender == owner(),\\n Errors.VL\\_NOT\\_AUTHORIZED\\n );\\n \\_;\\n}\\n```\\n\\nThe Cream Flash Loan initiation code requests the flash loan via ICTokenFlashloan(crToken).flashLoan(receiver=address(this), initiator=address(this), ...):\\n```\\n/\\*\\*\\n \\* @dev Initiates an CreamFinance flashloan.\\n \\* @param info: data to be passed between functions executing flashloan logic\\n \\*/\\nfunction \\_initiateCreamFlashLoan(FlashLoan.Info calldata info) internal {\\n address crToken = info.asset == \\_FTM\\n ? 0xd528697008aC67A21818751A5e3c58C8daE54696\\n : \\_crMappings.addressMapping(info.asset);\\n\\n // Prepara data for flashloan execution\\n bytes memory params = abi.encode(info);\\n\\n // Initialize Instance of Cream crLendingContract\\n ICTokenFlashloan(crToken).flashLoan(address(this), address(this), info.amount, params);\\n}\\n```\\n\\nNote: The Cream implementation does not send `sender=msg.sender` to the `onFlashLoan()` callback - like any other flash loan provider does and EIP-3156 suggests - but uses the value that was passed in as `initiator` when requesting the callback. This detail completely undermines the authentication checks implemented in `onFlashLoan` as the `sender` value cannot be trusted.\\n```\\naddress initiator,\\n```\\n\\n```\\n \\*/\\nfunction onFlashLoan(\\n address sender,\\n address underlying,\\n uint256 amount,\\n uint256 fee,\\n bytes calldata params\\n) external override returns (bytes32) {\\n // Check Msg. Sender is crToken Lending Contract\\n // from IronBank because ETH on Cream cannot perform a flashloan\\n address crToken = underlying == \\_WFTM\\n ? 0xd528697008aC67A21818751A5e3c58C8daE54696\\n : \\_crMappings.addressMapping(underlying);\\n require(msg.sender == crToken && address(this) == sender, Errors.VL\\_NOT\\_AUTHORIZED);\\n```\\n
Cream Finance\\nWe've reached out to the Cream developer team, who have confirmed the issue. They are planning to implement countermeasures. Our recommendation can be summarized as follows:\\nImplement the EIP-3156 compliant version of flashLoan() with initiator hardcoded to `msg.sender`.\\nFujiDAO (and other flash loan consumers)\\nWe recommend not assuming that `FlashLoan.Info` contains trusted or even validated data when a third-party flash loan provider provides it! Developers should ensure that the data received was provided when the flash loan was requested.\\nThe contract should reject unsolicited flash loans. In the scenario where a flash loan provider is exploited, the risk of an exploited trust relationship is less likely to spread to the rest of the system.\\nThe Cream `initiator` provided to the `onFlashLoan()` callback cannot be trusted until the Cream developers fix this issue. The `initiator` can easily be spoofed to perform unsolicited flash loans. We, therefore, suggest:\\nValidate that the `initiator` value is the `flashLoan()` caller. This conforms to the standard and is hopefully how the Cream team is fixing this, and\\nEnsure the implementation tracks its own calls to `flashLoan()` in a state-variable semaphore, i.e. store the flash loan data/hash in a temporary state-variable that is only set just before calling `flashLoan()` until being called back in `onFlashLoan()`. The received data can then be verified against the stored artifact. This is a safe way of authenticating and verifying callbacks.\\nValues received from untrusted third parties should always be validated with the utmost scrutiny.\\nSmart contract upgrades are risky, so we recommend implementing the means to pause certain flash loan providers.\\nEnsure that flash loan handler functions should never re-enter the system. This provides additional security guarantees in case a flash loan provider gets breached.\\nNote: The Fuji development team implemented a hotfix to prevent unsolicited calls from Cream by storing the `hash(FlashLoan.info)` in a state variable just before requesting the flash loan. Inside the `onFlashLoan` callback, this state is validated and cleared accordingly.\\nAn improvement to this hotfix would be, to check `_paramsHash` before any external calls are made and clear it right after validation at the beginning of the function. Additionally, `hash==0x0` should be explicitly disallowed. By doing so, the check also serves as a reentrancy guard and helps further reduce the risk of a potentially malicious flash loan re-entering the function.
```\\n/\\*\\*\\n \\* @dev Routing Function for Flashloan Provider\\n \\* @param info: struct information for flashLoan\\n \\* @param \\_flashnum: integer identifier of flashloan provider\\n \\*/\\nfunction initiateFlashloan(FlashLoan.Info calldata info, uint8 \\_flashnum) external isAuthorized override {\\n if (\\_flashnum == 0) {\\n \\_initiateGeistFlashLoan(info);\\n } else if (\\_flashnum == 2) {\\n \\_initiateCreamFlashLoan(info);\\n } else {\\n revert(Errors.VL\\_INVALID\\_FLASH\\_NUMBER);\\n }\\n}\\n```\\n