Calling redeem before claiming previous redeem tokens delay previous redeem batches too.

Summary

Dollars can be collected after a delay on redemption but calling redemption delays all previous redeems too.

Vulnerability Detail

Consider Alice has 1000 dollars to redeem and alice calls the redeemDollar function and redeem 500 dollar and let's say delay is of 1000 blocks.

But after the 1000 blocks have been added to chain, before calling collectRedemption(), alice decides to first call redeemDollar and redeem other 500 too.

The invariant that should hold is : ALice should be able to collect previous 500 Dai, that she is entitled to after redeeming 500 Dai (not considering fee here) as the 1000 blocks has been passed.

But according to implementation his previous payment have been delayed too, due to following line in redeemDollar(), when alice called that functions, it sets the variable to current block.number:

      poolStorage.lastRedeemedBlock[msg.sender] = block.number;

And following function fails in collectRedemption()

        require(
            (
                poolStorage.lastRedeemedBlock[msg.sender].add(
                    poolStorage.redemptionDelayBlocks
                )
            ) <= block.number,
            "Too soon to collect redemption"
        );

Impact

Redemption can be delayes.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L399-L517

Tool used

Cats

Recommendation

Change the code in a way where each redemption is tracked separately.

Duplicate of #180

sherlock-admin2 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

It's protocol decision