Prevent new users from registering (Zero Knowledge Proof Circuit)
Permanent freezing of unclaimed rewards
Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)
Contract fails to deliver promised returns, but doesn't lose value
Force DeGate into Exodus Mode
The account cannot be used (Zero Knowledge Proof Circuit)
Temporary freezing of funds: Minimum freezing of 15 days (Zero Knowledge Proof Circuit)
Description
Bug Description
The current ExchangeProxy at 0x9C07A72177c5A05410cA338823e790876E79D73B is managed by LoopringIOExchangeOwner0x9b93e47b7F61ad1358Bd47Cd01206708E85AE5eD, which is in turn controlled by an Externally Owned Account (EOA) 0xacD3A62F3eED1BfE4fF0eC8240d645c1F5477F82`. This setup poses significant security risks due to centralized control, allowing the key holder to unilaterally modify critical contract parameters and potentially shut down the system irreversibly.
Impact
Irreversible System Shutdown
The DeGate Operator, using an EOA, can entirely shut down the system. This can be executed by invoking Exchange.shutdown() through LoopringIOExchangeOwner.transact(abiEncodedCall), as demonstrated in the testExchangeShutdown test case.
Unilateral Modification of Critical Platform Configuration Parameters
In the testSetMaxAgeDepositUntilWithdrawable and testExchangeSetDepositParams tests, it's shown how the operator can change key Exchange settings via the same call chain initiated from their EOA.
Risk Breakdown
Difficulty to Exploit: Easy
Recommendation
To mitigate centralized control risks, Exchange's ownership (not just the proxy facade but also the business logic) should be restructured:
Transfer Ownership to Multisig: Change the ownership of LoopringIOExchangeOwner to a Gnosis Multisig wallet, distributing control among multiple parties.
Consider Using Timelock: Use Timelock contract as the owner to introduce a delay in critical operations, enhancing transparency.
Proof of concept
I have created a forge test, functioning on an Ethereum mainnet fork using the addreses of the live contracts. The file demonstrating vulnerabilities is MaliciousOperatorStopsExchangeContract.t.sol
Code (git)
commit 625637a9e64fa298b3a80b3216382c3382656f97
Author: Kirill Varlamov <kirill@ongrid.pro>
Date: Tue Nov 28 03:12:40 2023 +0400
test: malicious DeGate operator stops Exchange (centralized control by EOA)
diff --git a/foundry.toml b/foundry.toml
index 25b918f9..aa662d8a 100644
--- a/foundry.toml
+++ b/foundry.toml
@@ -3,4 +3,5 @@ src = "src"
out = "out"
libs = ["lib"]
+solc = '0.8.23'
# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options
diff --git a/src/Counter.sol b/src/Counter.sol
deleted file mode 100644
index aded7997..00000000
--- a/src/Counter.sol
+++ /dev/null
@@ -1,14 +0,0 @@
-// SPDX-License-Identifier: UNLICENSED
-pragma solidity ^0.8.13;
-
-contract Counter {
- uint256 public number;
-
- function setNumber(uint256 newNumber) public {
- number = newNumber;
- }
-
- function increment() public {
- number++;
- }
-}
diff --git a/test/Counter.t.sol b/test/Counter.t.sol
deleted file mode 100644
index e9b9e6ac..00000000
--- a/test/Counter.t.sol
+++ /dev/null
@@ -1,24 +0,0 @@
-// SPDX-License-Identifier: UNLICENSED
-pragma solidity ^0.8.13;
-
-import {Test, console2} from "forge-std/Test.sol";
-import {Counter} from "../src/Counter.sol";
-
-contract CounterTest is Test {
- Counter public counter;
-
- function setUp() public {
- counter = new Counter();
- counter.setNumber(0);
- }
-
- function test_Increment() public {
- counter.increment();
- assertEq(counter.number(), 1);
- }
-
- function testFuzz_SetNumber(uint256 x) public {
- counter.setNumber(x);
- assertEq(counter.number(), x);
- }
-}
diff --git a/test/MaliciousOperatorStopsExchangeContract.t.sol b/test/MaliciousOperatorStopsExchangeContract.t.sol
new file mode 100644
index 00000000..52685b34
--- /dev/null
+++ b/test/MaliciousOperatorStopsExchangeContract.t.sol
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.10;
+
+import "forge-std/Test.sol";
+import "test/mock/IExchangeV3.sol";
+import "test/mock/ILoopringIOExchangeOwner.sol";
+
+contract ShutdownExchangeTest is Test {
+ event DepositParamsUpdate(
+ uint256 freeDepositMax, uint256 freeDepositRemained, uint256 freeSlotPerBlock, uint256 depositFee
+ );
+
+ IExchangeV3 exchange;
+ address exchangeOwner;
+ ILoopringIOExchangeOwner ioex;
+ address ioexOwner;
+
+ function setUp() public {
+ uint256 fork;
+ fork = vm.createFork("https://mainnet.infura.io/v3/bcb7b03333384d49957cc9b4b53daa1d");
+ vm.selectFork(fork);
+ exchange = IExchangeV3(0x9C07A72177c5A05410cA338823e790876E79D73B);
+ ioex = ILoopringIOExchangeOwner(0x9b93e47b7F61ad1358Bd47Cd01206708E85AE5eD);
+ ioexOwner = ioex.owner();
+ }
+
+ function testLoopringIOExchangeOwnerIsEOA() public {
+ // ioexOwner is EOA (i.e. centralized control)
+ assertFalse(isContr
