LeverageActions DDoS protection

Gearbox-protocol · Oct 24, 2021 · 0b825ff · 0b825ff
1 parent 2a7a9c8
commit 0b825ff
Show file tree

Hide file tree

Showing 9 changed files with 149 additions and 51 deletions.
diff --git a/contracts/credit/CreditFilter.sol b/contracts/credit/CreditFilter.sol
@@ -87,8 +87,11 @@ contract CreditFilter is ICreditFilter, ACLTrait {
     // Maxmimum allowed fast check operations between full health factor checks
     uint256 public hfCheckInterval;
 
-   // Allowed transfers
-    mapping(address => mapping(address=> bool)) transfersAllowed;
+    // Allowed transfers
+    mapping(address => mapping(address => bool)) transfersAllowed;
+
+    // Allowed plugins
+    mapping(address => bool) public allowedPlugins;
 
     /// Checks that sender is connected credit manager
     modifier creditManagerOnly {
@@ -127,6 +130,8 @@ contract CreditFilter is ICreditFilter, ACLTrait {
             Constants.CHI_THRESHOLD,
             Constants.HF_CHECK_INTERVAL_DEFAULT
         ); // T:[CF-21]
+
+        allowedPlugins[addressProvider.getWETHGateway()] = true;
     }
 
     //
@@ -679,22 +684,37 @@ contract CreditFilter is ICreditFilter, ACLTrait {
         ); // T:[CM-28]
     }
 
-    function approveAccountTransfers(address from, bool state) external override {
-        transfersAllowed[from][msg.sender] = state;  // T:[CF-43]
+    function approveAccountTransfers(address from, bool state)
+        external
+        override
+    {
+        transfersAllowed[from][msg.sender] = state; // T:[CF-43]
         emit TransferAccountAllowed(from, msg.sender, state); // T:[CF-43]
     }
 
-    function allowanceForAccountTransfers(address from, address to) external view override returns(bool) {
-        return transfersAllowed[from][to];  // T:[CF-43]
+    function allowanceForAccountTransfers(address from, address to)
+        external
+        view
+        override
+        returns (bool)
+    {
+        return transfersAllowed[from][to]; // T:[CF-43]
+    }
+
+    function allowPlugin(address plugin, bool state) external configuratorOnly {
+        allowedPlugins[plugin] = state;
+        emit TransferPluginAllowed(plugin, state);
     }
 
     function revertIfAccountTransferIsNotAllowed(
         address owner,
         address newOwner
     ) external view override {
-        require(
-           transfersAllowed[owner][newOwner],
-            Errors.CF_TRANSFER_IS_NOT_ALLOWED
-        );  // T:[CF-43]
+        if (!allowedPlugins[owner] || allowedPlugins[newOwner]) {
+            require(
+                transfersAllowed[owner][newOwner],
+                Errors.CF_TRANSFER_IS_NOT_ALLOWED
+            ); // T:[CF-43, 44]
+        }
     }
 }
diff --git a/contracts/credit/CreditManager.sol b/contracts/credit/CreditManager.sol
@@ -179,10 +179,12 @@ contract CreditManager is ICreditManager, ACLTrait, ReentrancyGuard {
         ); // T:[CM-2]
 
         // Checks that user "onBehalfOf" has no opened accounts
-        require(
-            !hasOpenedCreditAccount(onBehalfOf) && onBehalfOf != address(0),
-            Errors.CM_ZERO_ADDRESS_OR_USER_HAVE_ALREADY_OPEN_CREDIT_ACCOUNT
-        ); // T:[CM-3]
+        //        require(
+        //            !hasOpenedCreditAccount(onBehalfOf) && onBehalfOf != address(0),
+        //            Errors.CM_ZERO_ADDRESS_OR_USER_HAVE_ALREADY_OPEN_CREDIT_ACCOUNT
+        //        ); // T:[CM-3]
+
+        _checkAccountTransfer(onBehalfOf);
 
         // borrowedAmount = amount * leverageFactor
         uint256 borrowedAmount = amount.mul(leverageFactor).div(
@@ -949,13 +951,22 @@ contract CreditManager is ICreditManager, ACLTrait, ReentrancyGuard {
         nonReentrant
     {
         address creditAccount = getCreditAccountOrRevert(msg.sender); // M:[LA-1,2,3,4,5,6,7,8] // T:[CM-52,53, 54]
-        require(
-            newOwner != address(0) && !hasOpenedCreditAccount(newOwner),
-            Errors.CM_INCORRECT_NEW_OWNER
-        ); // T:[CM-52,53]
-        creditFilter.revertIfAccountTransferIsNotAllowed(msg.sender, newOwner); // T:[54,55]
+        _checkAccountTransfer(newOwner);
         delete creditAccounts[msg.sender]; // T:[CM-54], M:[LA-1,2,3,4,5,6,7,8]
         creditAccounts[newOwner] = creditAccount; // T:[CM-54], M:[LA-1,2,3,4,5,6,7,8]
         emit TransferAccount(msg.sender, newOwner); // T:[CM-54]
     }
+
+    function _checkAccountTransfer(address newOwner) internal {
+        require(
+            newOwner != address(0) && !hasOpenedCreditAccount(newOwner),
+            Errors.CM_ZERO_ADDRESS_OR_USER_HAVE_ALREADY_OPEN_CREDIT_ACCOUNT
+        ); // T:[CM-52,53]
+        if (msg.sender != newOwner) {
+            creditFilter.revertIfAccountTransferIsNotAllowed(
+                msg.sender,
+                newOwner
+            ); // T:[54,55]
+        }
+    }
 }
diff --git a/contracts/interfaces/ICreditFilter.sol b/contracts/interfaces/ICreditFilter.sol
@@ -22,6 +22,12 @@ interface ICreditFilter {
         bool state
     );
 
+    event TransferPluginAllowed(
+        address indexed pugin,
+        bool state
+    );
+
+
     //
     // STATE-CHANGING FUNCTIONS
     //

diff --git a/test/credit.creditFilter.spec.ts b/test/credit.creditFilter.spec.ts
@@ -138,6 +138,10 @@ describe("CreditFilter", function () {
     await expect(
       creditFilter.connect(user).connectCreditManager(DUMB_ADDRESS)
     ).to.be.revertedWith(revertMsg);
+
+    await expect(
+      creditFilter.connect(user).allowPlugin(DUMB_ADDRESS, true)
+    ).to.be.revertedWith(revertMsg);
   });
 
   it("[CF-2]: allowToken, forbidToken reverts to add token with zero addresses", async () => {
@@ -1357,4 +1361,45 @@ describe("CreditFilter", function () {
       )
     ).to.be.revertedWith(revertMsg);
   });
+
+  it("[CF-44]: allowPlugin allows revertIfAccountTransferIsNotAllowed works without allowance", async () => {
+    const revertMsg = await errors.CF_TRANSFER_IS_NOT_ALLOWED();
+    await expect(
+      creditFilter.revertIfAccountTransferIsNotAllowed(
+        DUMB_ADDRESS,
+        deployer.address
+      )
+    ).to.be.revertedWith(revertMsg);
+
+    await expect(creditFilter.allowPlugin(DUMB_ADDRESS, true))
+      .to.emit(creditFilter, "TransferPluginAllowed")
+      .withArgs(DUMB_ADDRESS, true);
+
+    await expect(creditFilter.allowPlugin(DUMB_ADDRESS2, true))
+      .to.emit(creditFilter, "TransferPluginAllowed")
+      .withArgs(DUMB_ADDRESS2, true);
+
+    await creditFilter.revertIfAccountTransferIsNotAllowed(
+      DUMB_ADDRESS,
+      deployer.address
+    );
+
+    await expect(
+      creditFilter.revertIfAccountTransferIsNotAllowed(
+        DUMB_ADDRESS,
+        DUMB_ADDRESS2
+      )
+    ).to.be.revertedWith(revertMsg);
+
+    await expect(creditFilter.allowPlugin(DUMB_ADDRESS, false))
+      .to.emit(creditFilter, "TransferPluginAllowed")
+      .withArgs(DUMB_ADDRESS, false);
+
+    await expect(
+      creditFilter.revertIfAccountTransferIsNotAllowed(
+        DUMB_ADDRESS,
+        deployer.address
+      )
+    ).to.be.revertedWith(revertMsg);
+  });
 });
diff --git a/test/credit.creditManager.spec.ts b/test/credit.creditManager.spec.ts
@@ -338,12 +338,12 @@ describe("CreditManager", function () {
     await expect(
       creditManager
         .connect(user)
-        .openCreditAccount(amount, friend.address, leverageFactor, referral)
+        .openCreditAccount(amount, user.address, leverageFactor, referral)
     )
       .to.emit(creditManager, "OpenCreditAccount")
       .withArgs(
         user.address,
-        friend.address,
+        user.address,
         nextVA,
         amount,
         borrowedAmount,
@@ -1605,7 +1605,7 @@ describe("CreditManager", function () {
   });
 
   it("[CM-52]: transferAccountOwnership reverts for ZERO_ADDRESS", async () => {
-    const revertMsg = await errors.CM_INCORRECT_NEW_OWNER();
+    const revertMsg = await errors.CM_ZERO_ADDRESS_OR_USER_HAVE_ALREADY_OPEN_CREDIT_ACCOUNT();
     await ts.openDefaultCreditAccount();
 
     await expect(
@@ -1614,7 +1614,7 @@ describe("CreditManager", function () {
   });
 
   it("[CM-53]: transferAccountOwnership reverts for owner who has already credit account", async () => {
-    const revertMsg = await errors.CM_INCORRECT_NEW_OWNER();
+    const revertMsg = await errors.CM_ZERO_ADDRESS_OR_USER_HAVE_ALREADY_OPEN_CREDIT_ACCOUNT();
     await ts.openDefaultCreditAccount();
 
     await underlyingToken.approve(creditManager.address, MAX_INT);

diff --git a/test/mainnet/adapters.curve1.spec.ts b/test/mainnet/adapters.curve1.spec.ts
@@ -56,7 +56,7 @@ describe("CurveV1 adapter (Mainnet test)", function () {
 
     const testDeployer = new TestDeployer();
     errors = await testDeployer.getErrors();
-    const r1 = await ts.daiToken.approve(ts.creditManagerDAI.address, MAX_INT);
+    const r1 = await ts.daiToken.connect(user).approve(ts.creditManagerDAI.address, MAX_INT);
     await r1.wait();
     const r2 = await ts.daiToken.approve(ts.poolDAI.address, MAX_INT);
     await r2.wait();
@@ -76,15 +76,20 @@ describe("CurveV1 adapter (Mainnet test)", function () {
       );
       await r5.wait();
     }
+
+    const r6 = await ts.daiToken.transfer(user.address, accountAmount);
+    await r6.wait();
   });
 
   it("[CVA-1]: exchange works", async () => {
-    const r1 = await ts.creditManagerDAI.openCreditAccount(
-      accountAmount,
-      user.address,
-      leverageFactor,
-      referralCode
-    );
+    const r1 = await ts.creditManagerDAI
+      .connect(user)
+      .openCreditAccount(
+        accountAmount,
+        user.address,
+        leverageFactor,
+        referralCode
+      );
     await r1.wait();
 
     const amountOnAccount = accountAmount

diff --git a/test/mainnet/adapters.uniswapV2.spec.ts b/test/mainnet/adapters.uniswapV2.spec.ts
@@ -53,7 +53,7 @@ describe("UniswapV2 adapter (Mainnet test)", function () {
 
     const testDeployer = new TestDeployer();
     errors = await testDeployer.getErrors();
-    const r1 = await ts.daiToken.approve(ts.creditManagerDAI.address, MAX_INT);
+    const r1 = await ts.daiToken.connect(user).approve(ts.creditManagerDAI.address, MAX_INT);
     await r1.wait();
     const r2 = await ts.daiToken.approve(ts.poolDAI.address, MAX_INT);
     await r2.wait();
@@ -79,10 +79,8 @@ describe("UniswapV2 adapter (Mainnet test)", function () {
       .approve(UNISWAP_V2_ROUTER, MAX_INT);
     await r6.wait();
 
-    const r7 = await ts.daiToken
-      .transfer(user.address, accountAmount.mul(20));
+    const r7 = await ts.daiToken.transfer(user.address, accountAmount.mul(20));
     await r7.wait();
-
   });
 
   const openUserAccount = async () => {
@@ -102,12 +100,17 @@ describe("UniswapV2 adapter (Mainnet test)", function () {
       deployer
     );
 
-    const r1 = await ts.creditManagerDAI.openCreditAccount(
-      accountAmount,
-      user.address,
-      leverageFactor,
-      referralCode
-    );
+    const r0 = await ts.daiToken.transfer(user.address, accountAmount);
+    await r0.wait();
+
+    const r1 = await ts.creditManagerDAI
+      .connect(user)
+      .openCreditAccount(
+        accountAmount,
+        user.address,
+        leverageFactor,
+        referralCode
+      );
     await r1.wait();
 
     const creditAccount = await ts.creditManagerDAI.getCreditAccountOrRevert(
@@ -172,7 +175,7 @@ describe("UniswapV2 adapter (Mainnet test)", function () {
         UniV2helper.getDeadline()
       );
 
-    console.log("3")
+    console.log("3");
 
     expect(expectAmountsRouter).to.be.eql(expectAmountsAdapter);
 

diff --git a/test/mainnet/adapters.uniswapV3.spec.ts b/test/mainnet/adapters.uniswapV3.spec.ts
@@ -53,7 +53,7 @@ describe("UniswapV3 adapter (Mainnet test)", function () {
 
     const testDeployer = new TestDeployer();
     errors = await testDeployer.getErrors();
-    const r1 = await ts.daiToken.approve(ts.creditManagerDAI.address, MAX_INT);
+    const r1 = await ts.daiToken.connect(user).approve(ts.creditManagerDAI.address, MAX_INT);
     await r1.wait();
     const r2 = await ts.daiToken.approve(ts.poolDAI.address, MAX_INT);
     await r2.wait();
@@ -97,12 +97,17 @@ describe("UniswapV3 adapter (Mainnet test)", function () {
       deployer
     );
 
-    const r1 = await ts.creditManagerDAI.openCreditAccount(
-      accountAmount,
-      user.address,
-      leverageFactor,
-      referralCode
-    );
+    const r0 = await ts.daiToken.transfer(user.address, accountAmount);
+    await r0.wait();
+
+    const r1 = await ts.creditManagerDAI
+      .connect(user)
+      .openCreditAccount(
+        accountAmount,
+        user.address,
+        leverageFactor,
+        referralCode
+      );
     await r1.wait();
 
     const creditAccount = await ts.creditManagerDAI.getCreditAccountOrRevert(

diff --git a/test/mainnet/adapters.yearn.spec.ts b/test/mainnet/adapters.yearn.spec.ts
@@ -56,7 +56,7 @@ describe("YEARN adapter (Mainnet test)", function () {
 
     const testDeployer = new TestDeployer();
     errors = await testDeployer.getErrors();
-    const r1 = await ts.daiToken.approve(ts.creditManagerDAI.address, MAX_INT);
+    const r1 = await ts.daiToken.connect(user).approve(ts.creditManagerDAI.address, MAX_INT);
     await r1.wait();
     const r2 = await ts.daiToken.approve(ts.poolDAI.address, MAX_INT);
     await r2.wait();
@@ -94,8 +94,11 @@ describe("YEARN adapter (Mainnet test)", function () {
       deployer
     );
 
-    if (!(await ts.creditManagerDAI.hasOpenedCreditAccount(deployer.address))) {
-      const r1 = await ts.creditManagerDAI.openCreditAccount(
+    const r0 = await ts.daiToken.transfer(user.address, accountAmount);
+    await r0.wait()
+
+    if (!(await ts.creditManagerDAI.hasOpenedCreditAccount(user.address))) {
+      const r1 = await ts.creditManagerDAI.connect(user).openCreditAccount(
         accountAmount,
         user.address,
         leverageFactor,