fix: validate dispute module allowance

defi-wonderland · Jul 26, 2024 · 8f0beb1 · 8f0beb1
1 parent 43251e9
commit 8f0beb1
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 16 deletions.
diff --git a/solidity/contracts/extensions/AccountingExtension.sol b/solidity/contracts/extensions/AccountingExtension.sol
@@ -126,14 +126,13 @@ contract AccountingExtension is IAccountingExtension {
     uint256 _amount,
     address _sender
   ) external onlyAllowedModule(_requestId) onlyParticipant(_requestId, _bonder) {
-    if (!(_approvals[_bonder].contains(msg.sender) || _approvals[_bonder].contains(_sender))) {
+    bool _moduleApproved = _approvals[_bonder].contains(msg.sender);
+    bool _senderApproved = _bonder == _sender || _approvals[_bonder].contains(_sender);
+
+    if (!(_moduleApproved && _senderApproved)) {
       revert AccountingExtension_InsufficientAllowance();
     }
 
-    // if (!(_approvals[_bonder].contains(msg.sender) && (_bonder == _sender || _approvals[_bonder].contains(_sender)))) {
-    //   revert AccountingExtension_InsufficientAllowance();
-    // }
-
     if (balanceOf[_bonder][_token] < _amount) revert AccountingExtension_InsufficientFunds();
 
     bondedAmountOf[_bonder][_token][_requestId] += _amount;

diff --git a/solidity/test/integration/ResponseProposal.t.sol b/solidity/test/integration/ResponseProposal.t.sol
@@ -146,9 +146,6 @@ contract Integration_ResponseProposal is IntegrationBase {
       })
     );
 
-    uint256 _oldProposerBalance = _accountingExtension.balanceOf(proposer, usdc);
-    assertGt(_oldProposerBalance, 0);
-
     vm.startPrank(_attacker);
     // Attacker creates a request with their own address as the dispute module
     mockRequest.disputeModule = _attacker;
@@ -160,14 +157,8 @@ contract Integration_ResponseProposal is IntegrationBase {
     mockResponse.proposer = proposer;
     mockResponse.requestId = _requestIdAttacker;
 
+    // Should revert as the dispute module is not approved
+    vm.expectRevert(IAccountingExtension.AccountingExtension_InsufficientAllowance.selector);
     oracle.proposeResponse(mockRequest, mockResponse);
-
-    vm.stopPrank();
-
-    uint256 _newProposerBalance = _accountingExtension.balanceOf(proposer, usdc);
-
-    // Proposer got their balance bonded when they didn't create the response
-    assertTrue(_expectedBondSize != 0);
-    assertEq(_oldProposerBalance, _newProposerBalance + _expectedBondSize);
   }
 }