Merge pull request #2915 from rainlanguage/fix-2910-arb4-bad-raindex

arb4 rejects an untrusted raindex with BadRaindex

Author: thedavidmeister

.gas-snapshot

@@ -158,7 +158,6 @@ RaindexV6EnactTest:testRaindexEvalWriteStateDifferentOwnersNamespaced(address,ad
 RaindexV6EnactTest:testRaindexEvalWriteStateSequential() (gas: 556418)
 RaindexV6EnactTest:testRaindexEvalWriteStateSingle(address) (runs: 100, μ: 189480, ~: 189555)
 RaindexV6FlashBorrowerBadInitiatorTest:testOnFlashLoanBadInitiator(address) (runs: 5096, μ: 11005, ~: 11005)
-RaindexV6FlashBorrowerBadLenderApprovalTest:testBadLenderRevertsWithApproval() (gas: 4383922)
 RaindexV6FlashBorrowerFallbackTest:testFallbackAcceptsCalldata() (gas: 3149)
 RaindexV6FlashBorrowerFallbackTest:testFallbackAcceptsETHWithCalldata() (gas: 12730)
 RaindexV6FlashBorrowerFallbackTest:testFallbackAcceptsEmptyCalldata() (gas: 3113)

src/abstract/RaindexV6ArbCommon.sol

@@ -12,6 +12,13 @@ struct RaindexV6ArbConfig {
 /// Thrown when the task does not match the expected hash.
 error WrongTask();
 
+/// Thrown when an arb is called with a `raindex` that is not the trusted
+/// deterministic deployment. The arb grants `raindex` an unlimited token
+/// approval and calls it, so an untrusted address could drain the contract's
+/// balance.
+/// @param badRaindex The untrusted `raindex` passed to the arb.
+error BadRaindex(address badRaindex);
+
 /// @title RaindexV6ArbCommon
 /// @notice Common base for arb contracts that interact with `Raindex`.
 /// Provides a `_beforeArb` hook that is called at the start of every arb

src/abstract/RaindexV6ArbOrderTaker.sol

@@ -8,17 +8,12 @@ import {IERC20, SafeERC20} from "@openzeppelin-contracts-5.7.0/token/ERC20/utils
 import {IRaindexV6} from "raindex-interface-0.1.8/src/interface/IRaindexV6.sol";
 import {IRaindexV6ArbOrderTaker, TaskV2} from "raindex-interface-0.1.8/src/interface/IRaindexV6ArbOrderTaker.sol";
 import {TakeOrdersConfigV5, Float} from "raindex-interface-0.1.8/src/interface/IRaindexV6.sol";
-import {RaindexV6ArbCommon} from "./RaindexV6ArbCommon.sol";
+import {RaindexV6ArbCommon, BadRaindex} from "./RaindexV6ArbCommon.sol";
 import {LibRaindexArb} from "../lib/LibRaindexArb.sol";
 import {LibRaindexDeploy} from "../lib/deploy/LibRaindexDeploy.sol";
 import {IRaindexV6OrderTaker} from "raindex-interface-0.1.8/src/interface/IRaindexV6OrderTaker.sol";
 import {LibTOFUTokenDecimals} from "rain-tofu-erc20-decimals-0.1.4/src/lib/LibTOFUTokenDecimals.sol";
 
-/// Thrown when `arb5` is called with a `raindex` that is not the trusted
-/// deterministic deployment. `arb5` grants `raindex` an unlimited token approval
-/// and calls it, so an untrusted address could drain the contract's balance.
-error BadRaindex(address badRaindex);
-
 /// Thrown when `onTakeOrders2` is called during an arb by anything other than
 /// the trusted raindex.
 error UnexpectedOnTakeOrdersCaller(address caller);

src/abstract/RaindexV6FlashBorrower.sol

@@ -9,7 +9,7 @@ import {ReentrancyGuard} from "@openzeppelin-contracts-5.7.0/utils/ReentrancyGua
 import {ON_FLASH_LOAN_CALLBACK_SUCCESS} from "raindex-interface-0.1.8/src/interface/ierc3156/IERC3156FlashBorrower.sol";
 import {IRaindexV6, TakeOrdersConfigV5, TaskV2} from "raindex-interface-0.1.8/src/interface/IRaindexV6.sol";
 import {IERC3156FlashBorrower} from "raindex-interface-0.1.8/src/interface/ierc3156/IERC3156FlashBorrower.sol";
-import {RaindexV6ArbCommon} from "./RaindexV6ArbCommon.sol";
+import {RaindexV6ArbCommon, BadRaindex} from "./RaindexV6ArbCommon.sol";
 import {LibRaindexArb} from "../lib/LibRaindexArb.sol";
 import {LibRaindexDeploy} from "../lib/deploy/LibRaindexDeploy.sol";
 import {LibTOFUTokenDecimals} from "rain-tofu-erc20-decimals-0.1.4/src/lib/LibTOFUTokenDecimals.sol";
@@ -123,7 +123,8 @@ abstract contract RaindexV6FlashBorrower is IERC3156FlashBorrower, ReentrancyGua
     /// remaining assets are sent to the sender via `finalizeArb`.
     ///
     /// @param raindex The `Raindex` contract to take orders from and flash
-    /// loan against.
+    /// loan against. MUST be the deterministic deployment, else reverts
+    /// `BadRaindex`.
     /// @param takeOrders As per `IRaindexV6.takeOrders4`.
     /// @param exchangeData Arbitrary bytes that will be passed to `_exchange`
     /// after the flash loan is taken. The inheriting contract is responsible
@@ -142,6 +143,9 @@ abstract contract RaindexV6FlashBorrower is IERC3156FlashBorrower, ReentrancyGua
         if (takeOrders.orders.length == 0) {
             revert IRaindexV6.NoOrders();
         }
+        if (address(raindex) != LibRaindexDeploy.RAINDEX_DEPLOYED_ADDRESS) {
+            revert BadRaindex(address(raindex));
+        }
         // The flash-loan amount below reads `minimumIO` as the order output the
         // taker receives, which only matches when `minimumIO` is the taker
         // input (`IOIsInput == true`).

src/generated/candidate/GenericPoolRaindexV6FlashBorrower.sol

@@ -5,7 +5,7 @@ pragma solidity =0.8.25;
 import {GenericPoolRaindexV6ArbOrderTakerTest} from "test/util/abstract/GenericPoolRaindexV6ArbOrderTakerTest.sol";
 
 import {GenericPoolRaindexV6ArbOrderTaker} from "../../src/concrete/arb/GenericPoolRaindexV6ArbOrderTaker.sol";
-import {BadRaindex} from "../../src/abstract/RaindexV6ArbOrderTaker.sol";
+import {BadRaindex} from "../../src/abstract/RaindexV6ArbCommon.sol";
 import {LibRaindexDeploy} from "../../src/lib/deploy/LibRaindexDeploy.sol";
 import {
     IRaindexV6,

test/abstract/RaindexV6ArbOrderTaker.badRaindex.t.sol

@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: LicenseRef-DCL-1.0
+// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
+pragma solidity =0.8.25;
+
+import {Test} from "forge-std-1.17.0/src/Test.sol";
+import {IERC3156FlashLender} from "raindex-interface-0.1.8/src/interface/ierc3156/IERC3156FlashLender.sol";
+import {IRaindexV6} from "raindex-interface-0.1.8/src/interface/IRaindexV6.sol";
+import {BadRaindex} from "../../src/abstract/RaindexV6ArbCommon.sol";
+import {LibRaindexDeploy} from "../../src/lib/deploy/LibRaindexDeploy.sol";
+import {LibTestFlashBorrowerArb, FlashBorrowerSetup} from "test/util/lib/LibTestFlashBorrowerArb.sol";
+import {LibTestArb} from "test/util/lib/LibTestArb.sol";
+import {MockExchange} from "test/util/concrete/MockExchange.sol";
+
+contract RaindexV6FlashBorrowerBadRaindexTest is Test {
+    /// arb4 reverts BadRaindex for any raindex other than the deterministic
+    /// deployment, even a funded working lender, before calling it.
+    function testArb4RevertsUntrustedRaindex(address badRaindex) external {
+        vm.assume(badRaindex != LibRaindexDeploy.RAINDEX_DEPLOYED_ADDRESS);
+        vm.assume(badRaindex != address(0));
+        assumeNotPrecompile(badRaindex);
+        assumeNotForgeAddress(badRaindex);
+
+        MockExchange exchange = new MockExchange();
+        FlashBorrowerSetup memory setup = LibTestFlashBorrowerArb.setup(vm, address(exchange), 100e18);
+        vm.assume(badRaindex.code.length == 0);
+        vm.assume(badRaindex != address(setup.arb));
+        vm.etch(badRaindex, LibRaindexDeploy.RAINDEX_DEPLOYED_ADDRESS.code);
+        setup.outputToken.mint(badRaindex, 1000e18);
+
+        vm.expectCall(badRaindex, abi.encodeWithSelector(IERC3156FlashLender.flashLoan.selector), 0);
+        vm.expectRevert(abi.encodeWithSelector(BadRaindex.selector, badRaindex));
+        setup.arb.arb4(IRaindexV6(badRaindex), setup.takeOrdersConfig, setup.exchangeData, LibTestArb.noopTask());
+    }
+
+    /// arb4 reverts BadRaindex for the zero address.
+    function testArb4RevertsZeroRaindex() external {
+        MockExchange exchange = new MockExchange();
+        FlashBorrowerSetup memory setup = LibTestFlashBorrowerArb.setup(vm, address(exchange), 100e18);
+
+        vm.expectRevert(abi.encodeWithSelector(BadRaindex.selector, address(0)));
+        setup.arb.arb4(IRaindexV6(address(0)), setup.takeOrdersConfig, setup.exchangeData, LibTestArb.noopTask());
+    }
+}