The `builderReferral`, `purchaseReferral` and `deployer` can never be equal to `address(0)`, which leads to the `revolutionRewardRecipient` stealing their rewards

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/protocol-rewards/src/RevolutionProtocolRewards.sol#L110

Vulnerability details

HIGH

The builderReferral, purchaseReferral and deployer can never be equal to address(0), which leads to the revolutionRewardRecipient stealing their rewards

Description:

revolutionRewardRecipient will receive the rewards of the builderReferral, purchaseReferral and deployer anytime that their addresses equal address(0)

Proof of Concept:

builderReferral, purchaseReferral and deployer can never be equal to address(0) due to the fact that the function _depositPurchaseRewards in RewardSplits.sol explicitly checks for their address and sets it to the revolutionRewardRecipient's address if they are equal to address(0). After that the function calls RevolutionProtocolRewards::depositRewards. There is a check in the RevolutionProtocolRewards::depositRewards that checks all the builderReferral, purchaseReferral and deployer if they have an address different than address(0), and if they do, their addresses receive the rewards (all of their addresses will be set to the address of the revolutionRewardRecipient). That means that if their addresses are equal to address(0), then the address that receives all the rewards is the address of the revolutionRewardRecipient

RewardSplits.sol

        if (builderReferral == address(0)) builderReferral = revolutionRewardRecipient;

        if (deployer == address(0)) deployer = revolutionRewardRecipient;

        if (purchaseReferral == address(0)) purchaseReferral = revolutionRewardRecipient;
        .
        .
        .
        protocolRewards.depositRewards{ value: totalReward }(
            builderReferral,
            settings.builderReferralReward,
            purchaseReferral,
            settings.purchaseReferralReward,
            deployer,
            settings.deployerReward,
            revolutionRewardRecipient,
            settings.revolutionReward
        );

RevolutionProtocolRewards.sol

        unchecked {
            if (builderReferral != address(0)) balanceOf[builderReferral] += builderReferralReward;

            if (purchaseReferral != address(0)) balanceOf[purchaseReferral] += purchaseReferralReward;

            if (deployer != address(0)) balanceOf[deployer] += deployerReward;

            if (revolution != address(0)) balanceOf[revolution] += revolutionReward;
        }

Tools Used:

Manual Code Review.

Recommended Mitigation Steps:

Remove the if clause in RewardSplits::_depositPurchaseRewards so that if the addresses of the builderReferral, purchaseReferral or deployer are equal to address(0), their rewards will not be received by the revolutionRewardRecipient

-      if (builderReferral == address(0)) builderReferral = revolutionRewardRecipient;

-      if (deployer == address(0)) deployer = revolutionRewardRecipient;

-      if (purchaseReferral == address(0)) purchaseReferral = revolutionRewardRecipient;

Assessed type

Math

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Intended design.

[c4-sponsor]

rocketman-21 (sponsor) disputed

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid