Splitter deployment methodology will lead to race conditions for large portions of intial DAI distributions

Summary

Due to interdependent deployment requirements, the LockStake contracts, Uniswap migration contracts and new Splitter contracts must all be deployed simultaneously. Immediately following migration the excess DAI in the vat can be distributed to the farm. This allows users to deposit and distribute in the same block as deployment to claim excessive returns before others have the opportunity to deposit.

Vulnerability Detail

Splitter.sol#L57-L71

constructor(
    address _daiJoin
) {
    daiJoin = DaiJoinLike(_daiJoin);
    vat     = VatLike(daiJoin.vat());

    vat.hope(_daiJoin);

    hop  = 1 hours; // Initial value for safety

    wards[msg.sender] = 1;
    emit Rely(msg.sender);

    live = 1;
}

We see in the constructor of the flapper that zzz (the variable that tracks last distribution) is never initialized.

vow.sol#L148-L152

function flap() external returns (uint id) {
    require(vat.dai(address(this)) >= add(add(vat.sin(address(this)), bump), hump), "Vow/insufficient-surplus");
    require(sub(sub(vat.sin(address(this)), Sin), Ash) == 0, "Vow/debt-not-zero");
    id = flapper.kick(bump, 0);
}

We also see that in the vow, which is responsible for distributing protocol excess, does not have a built in timer and relies on the hop duration set in the splitter. This means that immediately after the splitter is migrated to, flap can immediately be called since zzz is never initialized.

StakingRewards.sol#L84-L90

function rewardPerToken() public view returns (uint256) {
    if (_totalSupply == 0) {
        return rewardPerTokenStored;
    }
    return
        rewardPerTokenStored + (((lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18) / _totalSupply);
}

We see in stakingRewards (the target of the burn), that rewards are distributed according to the totalSupply of deposited tokens. Due to these race conditions, even a very small deposit would net huge amounts of rewards from the contract before other depositors caught up and deposited their own tokens.

Impact

Race conditions allow first depositors to take large portions of DAI with very small deposits.

Code Snippet

FlapperUniV2.sol#L65-L86

Tool used

Manual Review

Recommendation

zzz should be initialized with a delay to let proper liquidity enter before allowing rewards to be distributed:

constructor(
    address _spotter,
    address _dai,
    address _gem,
    address _pair,
    address _receiver
++  uint256 _delay
) {
    spotter = SpotterLike(_spotter);

    dai = _dai;
    gem = _gem;
    require(GemLike(gem).decimals() == 18, "FlapperUniV2/gem-decimals-not-18");

    pair     = PairLike(_pair);
    daiFirst = pair.token0() == dai;
    receiver = _receiver;
++  zzz = block.timestamp + _delay

    wards[msg.sender] = 1;
    emit Rely(msg.sender);

    want = WAD; // Initial value for safety
}

sherlock-audit / 2024-06-makerdao-endgame-judging

0x52 - Splitter deployment methodology will lead to race conditions for large portions of intial DAI distributions #90