Arbitration competition changes

README.md

@@ -1,3 +1,11 @@
+# Audit Competition for HATs-Arbitration-Contracts
+This repository is for the audit competition for the HATs-Arbitration-Contracts.
+To participate, submit your findings only by using the on-chain submission process on https://app.hats.finance/vulnerability .
+## How to participate
+- follow the instructions on https://app.hats.finance/
+## Good luck!
+We look forward to seeing your findings.
+* * *
 # <img src="https://raw.githubusercontent.com/hats-finance/icons/main/hats.svg" alt="Hats.Finance" text="sds" height="40px"> Hats.Finance | Contracts
 
 [![Coverage Status](https://coveralls.io/repos/github/hats-finance/hats-contracts/badge.svg?t=Ko4Ndz&kill_cache=2)](https://coveralls.io/github/hats-finance/hats-contracts)

contracts/HATArbitrator.sol

@@ -25,6 +25,7 @@ contract HATArbitrator is IHATArbitrator, Ownable {
         public disputersBonds; // bonds provided by disputers
     mapping(address => mapping(IHATClaimsManager => mapping(bytes32 => bool)))
         public bondClaimable; // whether a given bond is reclaimable by the disputer
+    mapping(IHATClaimsManager => mapping(bytes32 => bool)) public claimDisputesDismissed; // claims of which disputes were dismissed
     mapping(IHATClaimsManager => mapping(bytes32 => uint256)) public totalBondsOnClaim; // total amount of bonds ona given claim
     mapping(IHATClaimsManager => mapping(bytes32 => Resolution)) public resolutions; // resolutions of disputes by the expert committee
     mapping(IHATClaimsManager => mapping(bytes32 => uint256))
@@ -160,6 +161,7 @@ contract HATArbitrator is IHATArbitrator, Ownable {
         onlyChallengedActiveClaim(_vault, _claimId)
         onlyUnresolvedDispute(_vault, _claimId)
     {
+        claimDisputesDismissed[_vault][_claimId] = true;
         resolutions[_vault][_claimId].resolvedAt = block.timestamp;
         token.safeTransfer(msg.sender, totalBondsOnClaim[_vault][_claimId]);
 
@@ -267,10 +269,14 @@ contract HATArbitrator is IHATArbitrator, Ownable {
     /** @notice See {IHATArbitrator-reclaimBond}. */
     function reclaimBond(IHATClaimsManager _vault, bytes32 _claimId) external {
         if (!bondClaimable[msg.sender][_vault][_claimId]) {
-            // the bond is claimable if either
+            // the bond is claimable if the claim wasn't dismissed and either
             // (a) it is not related to the current active claim
             // (b) it is about the current active claim but the claim has already expired
 
+            if (claimDisputesDismissed[_vault][_claimId]) {
+                revert ClaimDisputesDismissed();
+            }
+
             IHATClaimsManager.Claim memory claim = _vault.getActiveClaim();
 
             if (
@@ -482,6 +488,7 @@ contract HATArbitrator is IHATArbitrator, Ownable {
         ];
 
         if (
+            submitClaimRequest.submittedAt == 0 ||
             block.timestamp <=
             submitClaimRequest.submittedAt + submitClaimRequestReviewPeriod
         ) {
@@ -491,7 +498,7 @@ contract HATArbitrator is IHATArbitrator, Ownable {
         delete submitClaimRequests[_internalClaimId];
         token.safeTransfer(
             submitClaimRequest.submitter,
-            bondsNeededToStartDispute
+            submitClaimRequest.bond
         );
 
         emit SubmitClaimRequestExpired(_internalClaimId);

contracts/HATKlerosConnector.sol

@@ -164,8 +164,10 @@ contract HATKlerosConnector is IDisputeResolver, IHATKlerosConnector, Ownable {
         dispute.externalDisputeId = externalDisputeId;
         externalIDtoLocalID[externalDisputeId] = localDisputeId;
 
-        if (msg.value > arbitrationCost)
-            payable(_disputer).transfer(msg.value - arbitrationCost);
+        if (msg.value > arbitrationCost) {
+            (bool success,) = payable(_disputer).call{value: msg.value - arbitrationCost}("");
+            require(success, "Failed to send ETH");
+        }
 
         emit Challenged(_claimId);
         emit Dispute(
@@ -318,8 +320,11 @@ contract HATKlerosConnector is IDisputeResolver, IHATKlerosConnector, Ownable {
             );
         }
 
-        if (msg.value > contribution)
-            payable(msg.sender).transfer(msg.value - contribution); // Sending extra value back to contributor. It is the user's responsibility to accept ETH.
+        if (msg.value > contribution) {
+            (bool success,) = payable(msg.sender).call{value: msg.value - contribution}("");
+            require(success, "Failed to send ETH"); // Sending extra value back to contributor. It is the user's responsibility to accept ETH.
+        }
+
         return round.hasPaid[_side];
     }
 
@@ -362,7 +367,8 @@ contract HATKlerosConnector is IDisputeResolver, IHATKlerosConnector, Ownable {
 
         if (reward != 0) {
             round.contributions[_beneficiary][_side] = 0;
-            _beneficiary.transfer(reward); // It is the user's responsibility to accept ETH.
+            (bool success,) = payable(_beneficiary).call{value: reward}("");
+            require(success, "Failed to send ETH");  // It is the user's responsibility to accept ETH.
             emit Withdrawal(
                 _localDisputeId,
                 _round,

contracts/HATKlerosV2Connector.sol

@@ -97,7 +97,10 @@ contract HATKlerosV2Connector is IArbitrable, IHATKlerosConnector {
         dispute.externalDisputeId = externalDisputeId;
         externalIDtoLocalID[externalDisputeId] = localDisputeId;
 
-        if (msg.value > arbitrationCost) payable(_disputer).transfer(msg.value - arbitrationCost);
+        if (msg.value > arbitrationCost) {
+            (bool success,) = payable(_disputer).call{value: msg.value - arbitrationCost}("");
+            require(success, "Failed to send ETH");
+        }
 
         emit Challenged(_claimId);
         // TODO: add new IEvidence events once they're established.

contracts/HATPaymentSplitterFactory.sol

@@ -10,6 +10,12 @@ contract HATPaymentSplitterFactory {
     address public immutable implementation;
     event HATPaymentSplitterCreated(address indexed _hatPaymentSplitter);
 
+    error ArrayLengthMismatch();
+    error NoPayees();
+    error ZeroAddress();
+    error ZeroShares();
+    error DuplicatedPayee();
+
     constructor (address _implementation) {
         implementation = _implementation;
     }
@@ -21,6 +27,17 @@ contract HATPaymentSplitterFactory {
     }
 
     function predictSplitterAddress(address[] memory _payees, uint256[] memory _shares) public view returns (address) {
+        if (_payees.length != _shares.length) revert ArrayLengthMismatch();
+        if (_payees.length == 0) revert NoPayees();
+
+        for (uint256 i = 0; i < _payees.length; i++) {
+            if (_payees[i] == address(0)) revert ZeroAddress();
+            if (_shares[i] == 0) revert ZeroShares();
+            for (uint256 j = i + 1; j < _payees.length; j++) {
+                if (_payees[i] == _payees[j]) revert DuplicatedPayee();
+            }
+        }
+
         return Clones.predictDeterministicAddress(implementation, keccak256(abi.encodePacked(_payees, _shares)));
     }
 }

contracts/interfaces/IHATArbitrator.sol

@@ -25,6 +25,7 @@ interface IHATArbitrator {
     error AlreadyChallenged();
     error CourtCannotBeZero();
     error CannontChangeCourtAddress();
+    error ClaimDisputesDismissed();
 
     struct Resolution {
         address beneficiary;

contracts/interfaces/IHATClaimsManager.sol

@@ -176,7 +176,7 @@
     error NotEnoughUserBalance();
     // Only arbitrator or registry owner
     error OnlyArbitratorOrRegistryOwner();
-    // Unchalleged claim can only be approved if challenge period is over
+    // Unchallenged claim can only be approved if challenge period is over
     error UnchallengedClaimCanOnlyBeApprovedAfterChallengePeriod();
     // Challenged claim can only be approved by arbitrator before the challenge timeout period
     error ChallengedClaimCanOnlyBeApprovedByArbitratorUntilChallengeTimeoutPeriod();
@@ -488,5 +488,5 @@
     * @notice Returns the claims manager's version
     * @return The claims manager's version
     */
     function VERSION() external view returns(string calldata);
 }

contracts/mocks/BadKlerosConnectorEthReceiver.sol

@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.16;
+
+import "../interfaces/IHATArbitrator.sol";
+import "../HATKlerosConnector.sol";
+import "../interfaces/IHATClaimsManager.sol";
+
+contract BadKlerosConnectorEthReceiver {
+    bool public shouldFail = true;
+
+    function challengeResolution(
+        IHATArbitrator _arbitrator,
+        IHATClaimsManager _vault,
+        bytes32 _claimId,
+        string calldata _evidence
+    ) external payable {
+        _arbitrator.challengeResolution{value: msg.value}(_vault, _claimId, _evidence);
+    }
+
+    function fundAppeal(
+        HATKlerosConnector _connector,
+        uint256 _localDisputeId,
+        uint256 _side
+    ) external payable returns (bool) {
+        return _connector.fundAppeal{value: msg.value}(_localDisputeId, _side);
+    }
+
+    function setShouldFail(bool _shouldFail) external {
+        shouldFail = _shouldFail;
+    }
+
+    receive() external payable {
+        if (shouldFail) {
+            revert("cannot accept transfer");
+        }
+    }
+
+}

contracts/tokenlock/TokenLock.sol

@@ -42,6 +42,7 @@ abstract contract TokenLock is Ownable, ITokenLock {
     error AmountCannotBeZero();
     error AmountRequestedBiggerThanSurplus();
     error LockIsNonRevocable();
+    error LockIsAlreadyRevoked();
     error NoAvailableUnvestedAmount();
     error OnlySweeper();
     error CannotSweepVestedToken();
@@ -190,17 +191,27 @@ abstract contract TokenLock is Ownable, ITokenLock {
         if (!revocable)
             revert LockIsNonRevocable();
 
-        uint256 unvestedAmount = managedAmount - vestedAmount();
+        if (isRevoked)
+            revert LockIsAlreadyRevoked();
+
+        uint256 vestedAmount = vestedAmount();
+
+        uint256 unvestedAmount = managedAmount - vestedAmount;
         if (unvestedAmount == 0)
             revert NoAvailableUnvestedAmount();
 
         isRevoked = true;
 
+        managedAmount = vestedAmount;
+
+        // solhint-disable-next-line not-rely-on-time
+        endTime = block.timestamp;
+
         token.safeTransfer(owner(), unvestedAmount);
 
         emit TokensRevoked(beneficiary, unvestedAmount);
 
-        selfdestruct(payable(msg.sender));
+        trySelfDestruct();
     }
 
     /**
@@ -445,7 +456,7 @@ abstract contract TokenLock is Ownable, ITokenLock {
     }
 
     function trySelfDestruct() private {
-        if (currentTime() > endTime && currentBalance() == 0) {
+        if (currentTime() >= endTime && currentBalance() == 0) {
             selfdestruct(payable(msg.sender));
         }
     }

docs/dodoc/HATArbitrator.md

@@ -111,6 +111,29 @@ See {IHATArbitrator-challengeResolution}.
 | _claimId | bytes32 | undefined |
 | _evidence | string | undefined |
 
+### claimDisputesDismissed
+
+```solidity
+function claimDisputesDismissed(contract IHATClaimsManager, bytes32) external view returns (bool)
+```
+
+
+
+
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| _0 | contract IHATClaimsManager | undefined |
+| _1 | bytes32 | undefined |
+
+#### Returns
+
+| Name | Type | Description |
+|---|---|---|
+| _0 | bool | undefined |
+
 ### confiscateDisputers
 
 ```solidity
@@ -951,6 +974,17 @@ error ChallengePeriodPassed()
 
 
 
+### ClaimDisputesDismissed
+
+```solidity
+error ClaimDisputesDismissed()
+```
+
+
+
+
+
+
 ### ClaimExpired
 
 ```solidity

docs/dodoc/HATPaymentSplitterFactory.md

@@ -95,3 +95,61 @@ event HATPaymentSplitterCreated(address indexed _hatPaymentSplitter)
 
 
 
+## Errors
+
+### ArrayLengthMismatch
+
+```solidity
+error ArrayLengthMismatch()
+```
+
+
+
+
+
+
+### DuplicatedPayee
+
+```solidity
+error DuplicatedPayee()
+```
+
+
+
+
+
+
+### NoPayees
+
+```solidity
+error NoPayees()
+```
+
+
+
+
+
+
+### ZeroAddress
+
+```solidity
+error ZeroAddress()
+```
+
+
+
+
+
+
+### ZeroShares
+
+```solidity
+error ZeroShares()
+```
+
+
+
+
+
+
+

docs/dodoc/interfaces/IHATArbitrator.md

@@ -627,6 +627,17 @@ error ChallengePeriodPassed()
 
 
 
+### ClaimDisputesDismissed
+
+```solidity
+error ClaimDisputesDismissed()
+```
+
+
+
+
+
+
 ### ClaimExpired
 
 ```solidity