sherlock-audit / 2024-03-axis-finance-judging

1 stars 0 forks source link

web3tycoon - when a bidder calls `Refund`, and another bidder calls `refund` and frontruns the first `bidder`, The first transaction will fail. #172

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

web3tycoon

medium

when a bidder calls Refund, and another bidder calls refund and frontruns the first bidder, The first transaction will fail.

Summary

When bidder calls refund, and another bidder calls refund later, and his transaction goes through, while the first bidder, transaction is pending. his transaction may fail. This is because when he called refund the len had encaspulated the second bidder, therefore when his transaction runs, the length of the array cached will be greater than the actual queue length.

Vulnerability Detail

  1. There are 10 bidders in batch auction
  2. All of them call refund
  3. since this how we refund by using a loop

    
        uint256 len = bidIds.length;
        for (uint256 i; i < len; i++) {
            if (bidIds[i] == bidId_) {
                bidIds[i] = bidIds[len - 1];
                bidIds.pop();
                break;
            }
        }

    when lets say the transaction of the first bidder to call refund is logged after a later transaction, the transaction will revert, this is because length of the bidIds will have changed.

Impact

Denial of service leading a bidder to not get his refund back

Code Snippet

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/auctions/EMPAM.sol#L284

Tool used

Manual Review

Recommendation

we can use mappings, to avoid using a loop, can enums when refunding no other Bidder can enter.

Recommendation

nevillehuang commented 7 months ago

Invalid, no time sensitive reverts here. refund can still be performed subsequently