Lines of code

Vulnerability details

In the current design/implementation, the admin of BribeVault is a super privileged role of the system.

However, the inputs of the admin to some of the most critical methods are not being validated properly.

<https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L256-L260&gt;

This can lead to loss of funds to users caused by the admin’s misinput.

PoC

1. Alice depositBribeERC20() with 1,000 USDC for proposal-1;
2. Admin of BribeVault call transferBribes() for the proposal-1 with proof-1;
3. Bob depositBribeERC20() with 1,000 USDC for proposal-2;
4. Admin of BribeVault mistakenly call transferBribes() for the proposal-1 with proof-1 (should be proof-2);

Users who claimed rewards from proposal-1 can use the same proof and claim rewards from proposal-2.

Recommendation

1. Consider letting the brider setting the merkleRoot instead of the admin, see the Recommendation section in issue [WP-H3];
2. Consdier making bribeIdentifier part of the merkleProof;
3. Consider turning rewardToBribes to a mapping of struct of amount, token, and:

• adding rewardToBribes[bribeIdentifier].amount -= amount in RewardDistributor#_claim() to make sure the total claimed amount never excess the deposited reward amount;
• adding require(rewardToBribes[bribeIdentifier].token == _token) to make sure the token is correct;

The text was updated successfully, but these errors were encountered:

All reactions