code-423n4 / 2024-07-benddao-findings

9 stars 6 forks source link

isolateLiquidate() lock of check msgSender == lastBidder #47

Closed c4-bot-10 closed 3 months ago

c4-bot-10 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/logic/IsolateLogic.sol#L396

Vulnerability details

Vulnerability details

When the bidding ends, the user can call isolateLiquidate() to liquidate

  function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
...
    for (vars.nidx = 0; vars.nidx < params.nftTokenIds.length; vars.nidx++) {
      DataTypes.IsolateLoanData storage loanData = poolData.loanLookup[params.nftAsset][params.nftTokenIds[vars.nidx]];
      DataTypes.GroupData storage debtGroupData = debtAssetData.groupLookup[loanData.reserveGroup];
      DataTypes.ERC721TokenData storage tokenData = VaultLogic.erc721GetTokenData(
        nftAssetData,
        params.nftTokenIds[vars.nidx]
      );
...
      ValidateLogic.validateIsolateLiquidateLoan(params, debtGroupData, loanData);

      vars.auctionEndTimestamp = loanData.bidStartTimestamp + nftAssetData.auctionDuration;
      require(block.timestamp > vars.auctionEndTimestamp, Errors.ISOLATE_BID_AUCTION_DURATION_NOT_END);
@>  //@audit missing check msgSender == lastBidder
  function validateIsolateLiquidateLoan(
    InputTypes.ExecuteIsolateLiquidateParams memory inputParams,
    DataTypes.GroupData storage debtGroupData,
    DataTypes.IsolateLoanData storage loanData
  ) internal view {
    validateGroupBasic(debtGroupData);

    require(loanData.loanStatus == Constants.LOAN_STATUS_AUCTION, Errors.INVALID_LOAN_STATUS);
    require(loanData.reserveAsset == inputParams.asset, Errors.ISOLATE_LOAN_ASSET_NOT_MATCH);
  }

From the above code, we can see that this method does not restrict msgSender == lastBidder

It can be called by anyone, and will result in the winner of the bid, lastBidder, losing the corresponding bid funds.

Impact

The winner of the auction, lastBidder, loses the corresponding bid funds.

Recommended Mitigation

  function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
...
      vars.auctionEndTimestamp = loanData.bidStartTimestamp + nftAssetData.auctionDuration;
      require(block.timestamp > vars.auctionEndTimestamp, Errors.ISOLATE_BID_AUCTION_DURATION_NOT_END);
+     require(params.msgSender == loanData.lastBidder, " invald msgSender");

Assessed type

Context

c4-judge commented 3 months ago

MarioPoneder marked the issue as primary issue

c4-judge commented 3 months ago

MarioPoneder marked the issue as selected for report

c4-judge commented 3 months ago

MarioPoneder marked the issue as satisfactory

thorseldon commented 3 months ago

Fixed at https://github.com/BendDAO/bend-v2/commit/79c5e34248949871cae035c573ca256f3178da84.

But this finding is same with 14 (https://github.com/code-423n4/2024-07-benddao-findings/issues/14).

MarioPoneder commented 3 months ago

Thanks for pointing out the duplication!

c4-judge commented 3 months ago

MarioPoneder marked the issue as duplicate of #14

c4-judge commented 3 months ago

MarioPoneder marked the issue as not selected for report