fix: issue anyone can steal pledge

defi-wonderland · Jul 8, 2024 · 82a20f9 · 82a20f9
1 parent 55afd31
commit 82a20f9
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 1 deletion.
diff --git a/solidity/contracts/extensions/BondEscalationAccounting.sol b/solidity/contracts/extensions/BondEscalationAccounting.sol
@@ -17,7 +17,9 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
   mapping(bytes32 _disputeId => EscalationResult _result) public escalationResults;
 
   /// @inheritdoc IBondEscalationAccounting
-  mapping(bytes32 _requestId => mapping(address _pledger => bool)) public pledgerClaimed;
+  mapping(bytes32 _requestId => mapping(address _pledger => bool _claimed)) public pledgerClaimed;
+
+  mapping(bytes32 _disputeId => mapping(address _pledger => bool _pledged)) public pledgerHasPledged;
 
   constructor(IOracle _oracle) AccountingExtension(_oracle) {}
 
@@ -33,6 +35,8 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
 
     pledges[_disputeId][_token] += _amount;
 
+    pledgerHasPledged[_disputeId][_pledger] = true;
+
     unchecked {
       balanceOf[_pledger][_token] -= _amount;
     }
@@ -78,6 +82,7 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
     if (_result.token == IERC20(address(0))) revert BondEscalationAccounting_NoEscalationResult();
     bytes32 _requestId = _result.requestId;
     if (pledgerClaimed[_requestId][_pledger]) revert BondEscalationAccounting_AlreadyClaimed();
+    if (!pledgerHasPledged[_disputeId][_pledger]) revert BondEscalationAccounting_NotPledged();
 
     IOracle.DisputeStatus _status = ORACLE.disputeStatus(_disputeId);
     uint256 _amountPerPledger = _result.amountPerPledger;
@@ -95,6 +100,7 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
     uint256 _claimAmount = _amountPerPledger * _numberOfPledges;
 
     pledgerClaimed[_requestId][_pledger] = true;
+    pledgerHasPledged[_disputeId][_pledger] = false;
     balanceOf[_pledger][_token] += _claimAmount;
 
     unchecked {

diff --git a/solidity/interfaces/extensions/IBondEscalationAccounting.sol b/solidity/interfaces/extensions/IBondEscalationAccounting.sol
@@ -107,6 +107,11 @@ interface IBondEscalationAccounting is IAccountingExtension {
    */
   error BondEscalationAccounting_AlreadySettled();
 
+  /**
+   * @notice Thrown when an user tries to claim their pledge for an escalation which wasn't pledged
+   */
+  error BondEscalationAccounting_NotPledged();
+
   /*///////////////////////////////////////////////////////////////
                               STRUCTS
   //////////////////////////////////////////////////////////////*/
@@ -160,6 +165,15 @@ interface IBondEscalationAccounting is IAccountingExtension {
    */
   function pledgerClaimed(bytes32 _requestId, address _pledger) external returns (bool _claimed);
 
+  /**
+   * @notice True if the given pledger has pledged for the given dispute
+   *
+   * @param _disputeId         The ID of the bond-escalated dispute
+   * @param _pledger           Address of the pledger
+   * @return _pledged          True if the pledger has pledged
+   */
+  function pledgerHasPledged(bytes32 _disputeId, address _pledger) external returns (bool _pledged);
+
   /*///////////////////////////////////////////////////////////////
                               LOGIC
   //////////////////////////////////////////////////////////////*/

diff --git a/solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol b/solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol
@@ -19,6 +19,10 @@ import {Strings} from '@openzeppelin/contracts/utils/Strings.sol';
 contract ForTest_BondEscalationAccounting is BondEscalationAccounting {
   constructor(IOracle _oracle) BondEscalationAccounting(_oracle) {}
 
+  function forTest_setPledgerHasPledged(bytes32 _disputeId, address _pledger, bool _pledged) public {
+    pledgerHasPledged[_disputeId][_pledger] = _pledged;
+  }
+
   function forTest_setPledge(bytes32 _disputeId, IERC20 _token, uint256 _amount) public {
     pledges[_disputeId][_token] = _amount;
   }
@@ -445,6 +449,16 @@ contract BondEscalationAccounting_Unit_ClaimEscalationReward is BaseTest {
     bondEscalationAccounting.claimEscalationReward(_disputeId, pledger);
   }
 
+  function test_revertIfNotPledged(bytes32 _disputeId, bytes32 _requestId) public {
+    bondEscalationAccounting.forTest_setEscalationResult(
+      _disputeId, _requestId, token, 0, IBondEscalationModule(address(this))
+    );
+
+    vm.expectRevert(IBondEscalationAccounting.BondEscalationAccounting_NotPledged.selector);
+
+    bondEscalationAccounting.claimEscalationReward(_disputeId, pledger);
+  }
+
   function test_forVotesWon(
     bytes32 _disputeId,
     bytes32 _requestId,
@@ -462,6 +476,8 @@ contract BondEscalationAccounting_Unit_ClaimEscalationReward is BaseTest {
 
     bondEscalationAccounting.forTest_setPledge(_disputeId, token, _amount * _pledges);
 
+    bondEscalationAccounting.forTest_setPledgerHasPledged(_disputeId, pledger, true);
+
     // Mock and expect to call the oracle getting the dispute status
     _mockAndExpect(
       address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Won)
@@ -507,6 +523,8 @@ contract BondEscalationAccounting_Unit_ClaimEscalationReward is BaseTest {
 
     bondEscalationAccounting.forTest_setPledge(_disputeId, token, _amount * _pledges);
 
+    bondEscalationAccounting.forTest_setPledgerHasPledged(_disputeId, pledger, true);
+
     // Mock and expect to call the oracle getting the dispute status
     _mockAndExpect(
       address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Lost)