Stable Brick Swan - Wrong period considered in Pool::transferReserveToAuction

[sherlock-admin2]

Stable Brick Swan

High

Wrong period considered in Pool::transferReserveToAuction

Summary

The Pool::transferReserveToAuction function uses a wrong period to transfer the reserveToken amount to the auction. The proper period is the currentPeriod-1 while instead the function uses the currentPeriod.

Relevant GitHub Links

https://github.com/sherlock-audit/2024-12-plaza-finance/blob/main/plaza-evm/src/Pool.sol#L578-L579
https://github.com/sherlock-audit/2024-12-plaza-finance/blob/main/plaza-evm/src/BondToken.sol#L217-L229
https://github.com/sherlock-audit/2024-12-plaza-finance/blob/main/plaza-evm/src/Pool.sol#L567

Root Cause

The proper period to be used is the currentPeriod-1 because when creating an auction the current period in the bond contract increases by 1 cause of the function called BondToken::increaseIndexedAssetPeriod. So as it is every time the Pool::transferReserveToAuction function is called it will always get the address(0) as the auctions[currentPeriod].

function startAuction() external whenNotPaused() {
    // Check if distribution period has passed
    require(lastDistribution + distributionPeriod < block.timestamp, DistributionPeriodNotPassed());

    // Check if auction period hasn't passed
    require(lastDistribution + distributionPeriod + auctionPeriod >= block.timestamp, AuctionPeriodPassed());

    // Check if auction for current period has already started
    (uint256 currentPeriod,) = bondToken.globalPool();
    require(auctions[currentPeriod] == address(0), AuctionAlreadyStarted());

    uint8 bondDecimals = bondToken.decimals();
    uint8 sharesDecimals = bondToken.SHARES_DECIMALS();
    uint8 maxDecimals = bondDecimals > sharesDecimals ? bondDecimals : sharesDecimals;

    uint256 normalizedTotalSupply = bondToken.totalSupply().normalizeAmount(bondDecimals, maxDecimals);
    uint256 normalizedShares = sharesPerToken.normalizeAmount(sharesDecimals, maxDecimals);

    // Calculate the coupon amount to distribute
    uint256 couponAmountToDistribute = (normalizedTotalSupply * normalizedShares)
        .toBaseUnit(maxDecimals * 2 - IERC20(couponToken).safeDecimals());

    auctions[currentPeriod] = Utils.deploy(
      address(new Auction()),
      abi.encodeWithSelector(
        Auction.initialize.selector,
        address(couponToken),
        address(reserveToken),
        couponAmountToDistribute,
        block.timestamp + auctionPeriod,
        1000,
        address(this),
        poolSaleLimit
      )
    );

    // Increase the bond token period
 @> bondToken.increaseIndexedAssetPeriod(sharesPerToken);

    // Update last distribution time
    lastDistribution = block.timestamp;
  }

  function increaseIndexedAssetPeriod(uint256 sharesPerToken) public onlyRole(DISTRIBUTOR_ROLE) whenNotPaused() {
    globalPool.previousPoolAmounts.push(
      PoolAmount({
        period: globalPool.currentPeriod,
        amount: totalSupply(),
        sharesPerToken: globalPool.sharesPerToken
      })
    );
 @> globalPool.currentPeriod++;
    globalPool.sharesPerToken = sharesPerToken;

    emit IncreasedAssetPeriod(globalPool.currentPeriod, sharesPerToken);

function transferReserveToAuction(uint256 amount) external virtual {
 @>  (uint256 currentPeriod, ) = bondToken.globalPool();
 @> address auctionAddress = auctions[currentPeriod];
    require(msg.sender == auctionAddress, CallerIsNotAuction());
    
    IERC20(reserveToken).safeTransfer(msg.sender, amount);
  }

Internal Pre-conditions

An auction is created and ends with the sate SUCCEEDED.

External Pre-conditions

None.

Attack Path

The auction contract will try to call the Pool::transferReserveToAuction but it will not get the reserveToken amount because of the require. This because of the wrong period used in Pool::transferReserveToAuction.

function transferReserveToAuction(uint256 amount) external virtual {
 @>   (uint256 currentPeriod, ) = bondToken.globalPool();
 @>   address auctionAddress = auctions[currentPeriod];
 @>   require(msg.sender == auctionAddress, CallerIsNotAuction());  
      IERC20(reserveToken).safeTransfer(msg.sender, amount);
  }

Impact

Every auction that ends with the state SUCCEEDED will not be able to get the amount of the reserveToken it should.

Mitigation

function transferReserveToAuction(uint256 amount) external virtual {
      (uint256 currentPeriod, ) = bondToken.globalPool();
-     address auctionAddress = auctions[currentPeriod];
+     uint256 previousPeriod = currentPeriod - 1;
+     address auctionAddress = auctions[previousPeriod];
      require(msg.sender == auctionAddress, CallerIsNotAuction());  
      IERC20(reserveToken).safeTransfer(msg.sender, amount);
  }