Security: claimBounty() permissionless DoS + agentDid spoofing

BUG_REPORT_bounty_claim_dos.md

@@ -0,0 +1,94 @@
+# Bug: `claimBounty()` allows permissionless DoS of every open bounty
+
+**Severity**: HIGH — denial-of-service attack with zero cost.
+
+**Affected**: `src/GitlawbBounty.sol`, function `claimBounty(uint256, string)`
+
+## Root cause
+
+```solidity
+function claimBounty(
+    uint256 bountyId,
+    string calldata agentDid
+) external inStatus(bountyId, Status.Open) {
+    Bounty storage b = bounties[bountyId];
+    b.claimantDid = agentDid;
+    b.claimantAddress = msg.sender;
+    b.claimedAt = block.timestamp;
+    b.status = Status.Claimed;
+    emit BountyClaimed(bountyId, agentDid, msg.sender);
+}
+```
+
+No verification that :
+1. `agentDid` belongs to the caller (DID Registry binding check missing)
+2. The caller has any reputation / stake / eligibility
+3. The caller is even an agent (could be a random EOA)
+
+Any address can claim any open bounty for free.
+
+## Attack: indefinite DoS griefing
+
+1. Alice creates bounty (locks 1000 $GITLAWB)
+2. Mallory calls `claimBounty(bountyId, "fake-did")` — succeeds with no checks
+3. Mallory does nothing
+4. Anyone calls `disputeBounty()` after deadline → bounty reopens
+5. Mallory immediately calls `claimBounty()` again
+6. Repeat indefinitely
+
+Alice's 1000 $GITLAWB is locked forever in the contract. The only escape is `cancelBounty()` which refunds — but only works on `Status.Open`, so Alice must race Mallory to cancel between dispute and claim. Mallory can also send the cancel tx with higher gas to override.
+
+## Secondary issue: agentDid spoofing for reputation manipulation
+
+`agentDid` is a raw `string calldata`. On completion :
+
+```solidity
+bytes32 didHash = keccak256(bytes(b.claimantDid));
+agentEarnings[didHash] += payout;
+agentCompletedCount[didHash] += 1;
+```
+
+Earnings are credited to a DID hash that has NO relationship to the caller. Attacker can credit earnings to victim's DID OR steal credit for victim's work by colliding the DID string.
+
+## Suggested fix
+
+```solidity
+import { IGitlawbDIDRegistry } from "./interfaces/IGitlawbDIDRegistry.sol";
+
+IGitlawbDIDRegistry public immutable didRegistry;
+
+function claimBounty(
+    uint256 bountyId,
+    string calldata agentDid
+) external inStatus(bountyId, Status.Open) {
+    // Caller must control the DID (cryptographic binding via DID Registry)
+    require(
+        didRegistry.controllerOf(keccak256(bytes(agentDid))) == msg.sender,
+        "DID not owned by caller"
+    );
+    // (Optional) Require minimum stake or completed-count for anti-spam
+    // require(stakingV.getTier(msg.sender) >= Tier.LightNode, "below min tier");
+    Bounty storage b = bounties[bountyId];
+    b.claimantDid = agentDid;
+    b.claimantAddress = msg.sender;
+    b.claimedAt = block.timestamp;
+    b.status = Status.Claimed;
+    emit BountyClaimed(bountyId, agentDid, msg.sender);
+}
+```
+
+Or simpler if DID registry binding is not yet wired :
+
+```solidity
+require(
+    keccak256(abi.encodePacked(agentDid, msg.sender)) == /* expected commit */,
+    "agentDid commitment mismatch"
+);
+```
+
+Or simplest : require a small claim deposit (e.g. 100 $GITLAWB) refunded on `submitBounty` and slashed on dispute. Makes DoS economically infeasible.
+
+## Reporter
+
+@philpof102-svg — operator wallet `0xAC3ca7c5d3cDD7702fd08F9C4C28dAA22296aDa9` (Base)
+MainStreet: https://avisradar-production.up.railway.app/mainstreet.html

src/GitlawbBounty.sol

@@ -37,6 +37,7 @@ contract GitlawbBounty {
         uint256 submittedAt;
         uint256 completedAt;
         uint256 deadline;        // seconds from claim — auto-dispute after
+        uint256 claimDeposit;    // PR #6 : refundable deposit posted by claimant on claim
     }
 
     // ── Storage ──────────────────────────────────────────────────────────────
@@ -45,6 +46,8 @@ contract GitlawbBounty {
     address public treasury;
     address public owner;
     uint256 public protocolFeeBps; // basis points (500 = 5%)
+    /// PR #6 fix : refundable claim deposit (basis points of bounty amount)
+    uint256 public claimDepositBps;
     uint256 public nextBountyId;
     uint256 public defaultDeadline; // seconds (default 7 days)
 
@@ -69,6 +72,7 @@ contract GitlawbBounty {
     event BountyDisputed(uint256 indexed bountyId);
     event TreasuryUpdated(address indexed newTreasury);
     event FeeUpdated(uint256 newFeeBps);
+    event ClaimDepositBpsUpdated(uint256 newBps); // PR #6
 
     // ── Errors ───────────────────────────────────────────────────────────────
 
@@ -106,6 +110,7 @@ contract GitlawbBounty {
         treasury = _treasury;
         owner = msg.sender;
         protocolFeeBps = 500; // 5%
+        claimDepositBps = 100; // 1% refundable deposit (PR #6)
         defaultDeadline = 7 days;
     }
 
@@ -154,10 +159,23 @@ contract GitlawbBounty {
         string calldata agentDid
     ) external inStatus(bountyId, Status.Open) {
         Bounty storage b = bounties[bountyId];
+
+        // PR #6 fix : require refundable claim deposit. Stops the
+        // indefinite DoS where any address claims any bounty for free,
+        // lets the deadline expire, and re-claims after dispute.
+        // Deposit is refunded on submitBounty, slashed to treasury on
+        // disputeBounty (claimant missed deadline).
+        uint256 deposit = (b.amount * claimDepositBps) / 10_000;
+        if (deposit > 0) {
+            bool dok = token.transferFrom(msg.sender, address(this), deposit);
+            if (!dok) revert TransferFailed();
+        }
+
         b.claimantDid = agentDid;
         b.claimantAddress = msg.sender;
         b.claimedAt = block.timestamp;
         b.status = Status.Claimed;
+        b.claimDeposit = deposit;
 
         emit BountyClaimed(bountyId, agentDid, msg.sender);
     }
@@ -175,6 +193,14 @@ contract GitlawbBounty {
         b.submittedAt = block.timestamp;
         b.status = Status.Submitted;
 
+        // PR #6 fix : refund the claim deposit on timely submission.
+        if (b.claimDeposit > 0) {
+            uint256 refund = b.claimDeposit;
+            b.claimDeposit = 0;
+            bool rok = token.transfer(msg.sender, refund);
+            if (!rok) revert TransferFailed();
+        }
+
         emit BountySubmitted(bountyId, prId);
     }
 
@@ -237,6 +263,15 @@ contract GitlawbBounty {
             revert DeadlineNotExceeded(bountyId);
         }
 
+        // PR #6 fix : slash the claim deposit to treasury since the claimant
+        // missed the deadline. Makes spam-claiming economically lossy.
+        if (b.claimDeposit > 0 && treasury != address(0)) {
+            uint256 slashed = b.claimDeposit;
+            b.claimDeposit = 0;
+            bool sok = token.transfer(treasury, slashed);
+            if (!sok) revert TransferFailed();
+        }
+
         b.status = Status.Open;
         b.claimantDid = "";
         b.claimantAddress = address(0);
@@ -301,6 +336,12 @@ contract GitlawbBounty {
         emit TreasuryUpdated(_treasury);
     }
 
+    function setClaimDepositBps(uint256 _bps) external onlyOwner {
+        require(_bps <= 2000, "deposit too high"); // max 20% of bounty
+        claimDepositBps = _bps;
+        emit ClaimDepositBpsUpdated(_bps);
+    }
+
     function setProtocolFee(uint256 _feeBps) external onlyOwner {
         require(_feeBps <= 1000, "fee too high"); // max 10%
         protocolFeeBps = _feeBps;