THE EXPLOIT
We added a bountyExists check in DepositManager::fundBountyToken to prevent event spoofing. This is good.
We DID NOT add the same bountyExists protection on ClaimManagerV1::permissionedClaimTieredBounty(address,bytes) or ClaimManagerV1::claimBounty.
This would cause spoofed ClaimSuccess, TokenBalanceClaimed, and BountyClosed events to be emitted.
It DOES NOT risk funds since the transfer methods only transfers the bounty addresses balance, which the attacker would not control
THE FIX
Add the same bountyExists check to permissionedClaimTieredBounty(address,bytes) and claimBounty(address,address,bytes)
The best practice violation was just a missing onlyProxy on the ClaimManager pause methods
THE IMPACT
-
Our subgraph used in the frontend consumes these events. Data inconsistency can nuke much of our frontend logic.
-
We send emails to both claimants and event organizers based off of these events
THE EXPLOIT
We added a
bountyExistscheck inDepositManager::fundBountyTokento prevent event spoofing. This is good.We DID NOT add the same
bountyExistsprotection onClaimManagerV1::permissionedClaimTieredBounty(address,bytes)orClaimManagerV1::claimBounty.This would cause spoofed
ClaimSuccess,TokenBalanceClaimed, andBountyClosedevents to be emitted.It DOES NOT risk funds since the transfer methods only transfers the bounty addresses balance, which the attacker would not control
THE FIX
Add the same
bountyExistscheck topermissionedClaimTieredBounty(address,bytes)andclaimBounty(address,address,bytes)The best practice violation was just a missing
onlyProxyon theClaimManagerpausemethodsTHE IMPACT
Our subgraph used in the frontend consumes these events. Data inconsistency can nuke much of our frontend logic.
We send emails to both claimants and event organizers based off of these events