From d8c24715ea3559787c5d564fa8d6f5b8f12e0ecb Mon Sep 17 00:00:00 2001
From: Lukasz Zimnoch <lukasz.zimnoch@keep.network>
Date: Thu, 14 Dec 2023 13:50:47 +0100
Subject: [PATCH] Require ECDSA DKG result challenger to be an EOA

The `challengeDkgResult` function uses several `try-catch` blocks as part
of its business logic. However, the EVM has a call stack depth limit equal to
1024. A third-party contract can leverage this limitation and force the
`try-catch`-ed calls to revert unconditionally, by using recursion and
letting those calls to be executed at depth 1025. In such a case, the
control flow is passed to the `catch` clauses which may lead to some undesired
side effects like invalidation of a proper DKG result. To address that problem,
we are adding a requirement that `challengeDkgResult` can be only called by
an EOA. This prevents third-party contracts against calling
`challengeDkgResult`.
---
 solidity/ecdsa/contracts/WalletRegistry.sol   |  3 +++
 .../ecdsa/contracts/test/DkgChallenger.sol    | 18 +++++++++++++++
 .../WalletRegistry.WalletCreation.test.ts     | 22 +++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 solidity/ecdsa/contracts/test/DkgChallenger.sol

diff --git a/solidity/ecdsa/contracts/WalletRegistry.sol b/solidity/ecdsa/contracts/WalletRegistry.sol
index b77fa133d9..94882c7de6 100644
--- a/solidity/ecdsa/contracts/WalletRegistry.sol
+++ b/solidity/ecdsa/contracts/WalletRegistry.sol
@@ -800,6 +800,9 @@ contract WalletRegistry is
     ///      requires an extra amount of gas to be left at the end of the
     ///      execution.
     function challengeDkgResult(DKG.Result calldata dkgResult) external {
+        // solhint-disable-next-line avoid-tx-origin
+        require(msg.sender == tx.origin, "Not EOA");
+
         (
             bytes32 maliciousDkgResultHash,
             uint32 maliciousDkgResultSubmitterId
diff --git a/solidity/ecdsa/contracts/test/DkgChallenger.sol b/solidity/ecdsa/contracts/test/DkgChallenger.sol
new file mode 100644
index 0000000000..5b3e36f79a
--- /dev/null
+++ b/solidity/ecdsa/contracts/test/DkgChallenger.sol
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-3.0-only
+
+pragma solidity 0.8.17;
+
+import "../WalletRegistry.sol";
+import "../libraries/EcdsaDkg.sol";
+
+contract DkgChallenger {
+    WalletRegistry internal walletRegistry;
+
+    constructor(WalletRegistry _walletRegistry) {
+        walletRegistry = _walletRegistry;
+    }
+
+    function challengeDkgResult(EcdsaDkg.Result calldata dkgResult) external {
+        walletRegistry.challengeDkgResult(dkgResult);
+    }
+}
diff --git a/solidity/ecdsa/test/WalletRegistry.WalletCreation.test.ts b/solidity/ecdsa/test/WalletRegistry.WalletCreation.test.ts
index 4f9373038d..7817b48987 100644
--- a/solidity/ecdsa/test/WalletRegistry.WalletCreation.test.ts
+++ b/solidity/ecdsa/test/WalletRegistry.WalletCreation.test.ts
@@ -26,6 +26,7 @@ import type {
   WalletRegistryStub,
   TokenStaking,
   IRandomBeacon,
+  DkgChallenger,
 } from "../typechain"
 import type { DkgResult, DkgResultSubmittedEventArgs } from "./utils/dkg"
 import type { Operator } from "./utils/operators"
@@ -2244,6 +2245,27 @@ describe("WalletRegistry - Wallet Creation", async () => {
   })
 
   describe("challengeDkgResult", async () => {
+    context("with caller being a contract", async () => {
+      let dkgChallenger: DkgChallenger
+
+      before("request new wallet", async () => {
+        await createSnapshot()
+
+        const DkgChallenger = await ethers.getContractFactory("DkgChallenger")
+        dkgChallenger = await DkgChallenger.deploy(walletRegistry.address)
+      })
+
+      after(async () => {
+        await restoreSnapshot()
+      })
+
+      it("should revert", async () => {
+        await expect(
+          dkgChallenger.challengeDkgResult(stubDkgResult)
+        ).to.be.revertedWith("Not EOA")
+      })
+    })
+
     context("with no wallets registered", async () => {
       it("should revert with 'Current state is not CHALLENGE'", async () => {
         await expect(