Skip to content

Security MEDIUM-HIGH : Bounty retroactive fee bump rugpulls escrowed bounties#11

Open
philpof102-svg wants to merge 2 commits into
Gitlawb:mainfrom
philpof102-svg:fix-bounty-retroactive-fee
Open

Security MEDIUM-HIGH : Bounty retroactive fee bump rugpulls escrowed bounties#11
philpof102-svg wants to merge 2 commits into
Gitlawb:mainfrom
philpof102-svg:fix-bounty-retroactive-fee

Conversation

@philpof102-svg

Copy link
Copy Markdown

approveBounty reads CURRENT protocolFeeBps instead of fee-at-creation. Owner front-runs an inflight approval with setProtocolFee(1000) → claimant share drops 5% → owner pockets the delta on a bounty they had no role in creating.

Same class as variable-rate-without-notice. Fix : snapshot feeBpsAtCreation in createBounty, use it in approveBounty.

Full PoC + fix code in BUG_REPORT_bounty_retroactive_fee.md.

7th PR in continuous Gitlawb audit. Related : #5, #6, #7, #8, #9, #10. Reporter @philpof102-svg.

@philpof102-svg

Copy link
Copy Markdown
Author

Upgraded with merge-ready fix (src/GitlawbBounty.sol, +7/-2).

Minimal diff : snapshot protocolFeeBps at createBounty() and use it at approveBounty() instead of the live value.

struct Bounty { ...; uint256 feeBpsSnapshot; }

function createBounty(...) external returns (uint256) {
    ...
    bounties[bountyId] = Bounty({
        ...
        feeBpsSnapshot: protocolFeeBps // locked at escrow time
    });
}

function approveBounty(uint256 bountyId) external ... {
    Bounty storage b = bounties[bountyId];
    uint256 fee = (b.amount * b.feeBpsSnapshot) / 10000; // was: protocolFeeBps
    ...
}

Matches how every mature escrow / lending protocol handles rate changes (Aave, Compound, Morpho, Uniswap fee tiers — all snapshot at obligation time). Bounty creators get a guarantee that the agent's payout = amount * (10000 - feeAtCreation) / 10000.

Backward compatible : pre-fix bounties have feeBpsSnapshot == 0, which means they pay 0% fee on approval. Conservative default — does NOT retroactively tax in-flight bounties (the whole point of this PR).

…ps (PR Gitlawb#11)

Previously approveBounty() computed the protocol fee using the CURRENT
protocolFeeBps. Combined with the owner's ability to call setProtocolFee
at any time, this meant the owner could rugpull every in-flight escrowed
bounty by bumping the fee to 100% between submitBounty() and
approveBounty() — the agent's payout collapses to 0, all funds route to
treasury.

Fix : Bounty struct gains feeBpsSnapshot, locked to protocolFeeBps at
createBounty() time. approveBounty() uses that snapshot. Future fee
changes apply only to bounties created AFTER the change.

This matches how every escrow / lending protocol handles fee changes
(Aave, Compound, Morpho, Uniswap all snapshot the rate at the moment
of obligation). Bounty creators get a guarantee that the agent payout
will be (amount * (10000 - feeAtCreation)) / 10000, full stop.

Backward compatible : existing in-flight bounties have feeBpsSnapshot=0
in storage which gives them a 0% fee (favourable to the agent). The
only path that hits that is approveBounty() on bounties created BEFORE
the upgrade — defensive default is to NOT retroactively tax them.

Related to PR Gitlawb#11 bug report.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant