diff --git a/BUG_REPORT_bounty_claim_dos.md b/BUG_REPORT_bounty_claim_dos.md new file mode 100644 index 0000000..f9336f7 --- /dev/null +++ b/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 diff --git a/src/GitlawbBounty.sol b/src/GitlawbBounty.sol index 0ef28f1..06eedd6 100644 --- a/src/GitlawbBounty.sol +++ b/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;