code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

Same tokens added to bounty #253

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

goatbug

Vulnerability details

Impact

Even if the same token is added as a bounty, they will be treated as seperate bounties and require 2 transfers of the same token to claim.

function addBounty(IERC20 token, uint256 amount) public override returns (uint256) {
    // add bounty to basket
    token.safeTransferFrom(msg.sender, address(this), amount);
    _bounties.push(Bounty({
        token: address(token),
        amount: amount,
        active: true
    }));

    uint256 id = _bounties.length - 1;
    emit BountyAdded(token, amount, id);
    return id;
}

Proof of Concept

Another optimization: Bounties further keep being added to the list and never deleted. Gas could be saved by removing bounties instead of setting their active flag to false.

Tools Used

Recommended Mitigation Steps

GalloDaSballo commented 3 years ago

Agree with the finding, it may be best to check for uniqueness (for example by using an EnumerableMap) As per the second finding, that's correct as setting storage to 0 will trigger gas refunds (up to 1/5 of the cost of the tx)