Summary

The PreMarktes contract is vulnerable to reentrancy attacks due to the lack of reentrancy guards in the createOffer and closeOffer functions. An attacker can exploit this by recursively calling the vulnerable functions, causing inconsistent contract state and potential financial loss.

Vulnerability Details

The vulnerability lies in the createOffer and closeOffer functions, which transfer tokens before updating the contract state. Specifically, the external call to tokenManager.tillIn can be exploited by a malicious contract to reenter the createOffer function before the state variables are updated.

Impact

An attacker can exploit this vulnerability to:

• Reenter the createOffer function, creating multiple offers without the expected constraints.

• Drain funds from the contract or cause financial loss to other users by manipulating the contract state.

Tools Used

Manual

Recommendations

• Implement Reentrancy Guard: Add the nonReentrant modifier to the createOffer and closeOffer functions to prevent reentrancy attacks.

• Reorder State Updates and External Calls: Ensure that all state updates occur before any external calls to mitigate the risk of reentrancy.

  Before Changes :

  function createOffer(CreateOfferParams calldata params) external payable {

  // ... existing code ...

  ​

  // Increment the offer ID

  offerId++;

  ​

  // Transfer collateral from the sender to the capital pool

  uint256 transferAmount = OfferLibraries.getDepositAmount(

  params.offerType,

  params.collateralRate,

  params.amount,

  true,

  Math.Rounding.Ceil

  );

  ​

  ITokenManager tokenManager = tadleFactory.getTokenManager();

  tokenManager.tillIn{value: msg.value}(_msgSender(), params.tokenAddress, transferAmount, false);

  ​

  // Update maker info

  makerInfoMap[makerAddr] = MakerInfo({

  offerSettleType: params.offerSettleType,

  authority: _msgSender(),

  marketPlace: params.marketPlace,

  tokenAddress: params.tokenAddress,

  originOffer: offerAddr,

  platformFee: 0,

  eachTradeTax: params.eachTradeTax

  });

  ​

  // Update offer info

  offerInfoMap[offerAddr] = OfferInfo({

  id: offerId,

  authority: _msgSender(),

  maker: makerAddr,

  offerStatus: OfferStatus.Virgin,

  offerType: params.offerType,

  points: params.points,

  amount: params.amount,

  collateralRate: params.collateralRate,

  abortOfferStatus: AbortOfferStatus.Initialized,

  usedPoints: 0,

  tradeTax: 0,

  settledPoints: 0,

  settledPointTokenAmount: 0,

  settledCollateralAmount: 0

  });

  ​

  // Update stock info

  stockInfoMap[stockAddr] = StockInfo({

  id: offerId,

  stockStatus: StockStatus.Initialized,

  stockType: params.offerType == OfferType.Ask ? StockType.Bid : StockType.Ask,

  authority: _msgSender(),

  maker: makerAddr,

  preOffer: address(0),

  offer: offerAddr,

  points: params.points,

  amount: params.amount

  });

  ​

  // Emit event for creating offer

  emit CreateOffer(offerAddr, makerAddr, stockAddr, params.marketPlace, _msgSender(), params.points, params.amount);

  }

  After Changes:

  function createOffer(CreateOfferParams calldata params) external payable nonReentrant {

  // ... existing code ...

  // Increment the offer ID

  offerId++;

  ​

  // Update maker info

  makerInfoMap[makerAddr] = MakerInfo({

  offerSettleType: params.offerSettleType,

  authority: _msgSender(),

  marketPlace: params.marketPlace,

  tokenAddress: params.tokenAddress,

  originOffer: offerAddr,

  platformFee: 0,

  eachTradeTax: params.eachTradeTax

  });

  ​

  // Update offer info

  offerInfoMap[offerAddr] = OfferInfo({

  id: offerId,

  authority: _msgSender(),

  maker: makerAddr,

  offerStatus: OfferStatus.Virgin,

  offerType: params.offerType,

  points: params.points,

  amount: params.amount,

  collateralRate: params.collateralRate,

  abortOfferStatus: AbortOfferStatus.Initialized,

  usedPoints: 0,

  tradeTax: 0,

  settledPoints: 0,

  settledPointTokenAmount: 0,

  settledCollateralAmount: 0

  });

  ​

  // Update stock info

  stockInfoMap[stockAddr] = StockInfo({

  id: offerId,

  stockStatus: StockStatus.Initialized,

  stockType: params.offerType == OfferType.Ask ? StockType.Bid : StockType.Ask,

  authority: _msgSender(),

  maker: makerAddr,

  preOffer: address(0),

  offer: offerAddr,

  points: params.points,

  amount: params.amount

  });

  ​

  // Transfer collateral from the sender to the capital pool

  uint256 transferAmount = OfferLibraries.getDepositAmount(

  params.offerType,

  params.collateralRate,

  params.amount,

  true,

  Math.Rounding.Ceil

  );

  ​

  ITokenManager tokenManager = tadleFactory.getTokenManager();

  tokenManager.tillIn{value: msg.value}(_msgSender(), params.tokenAddress, transferAmount, false);

  }

Detailed Summary of the Changes:

1. Adding the nonReentrant Modifier:

   • Before: The createOffer function did not have any protection against reentrancy.

   • After: Adding the nonReentrant modifier ensures that the function cannot be called recursively, preventing reentrancy attacks.

2. Reordering State Updates and External Calls:

   • Before: The state variables (like offerId, makerInfoMap, offerInfoMap, and stockInfoMap) were updated after the external call to tokenManager.tillIn.

   • After: The state variables are updated before the external call to tokenManager.tillIn.

How It Causes Fund Loss:

• Reentrancy Attack: Without the nonReentrant modifier, an attacker can exploit the external call within the createOffer function to reenter the function before the state updates are finalized. This reentrancy can allow the attacker to manipulate the state or drain funds.

• Impact on Funds: The attacker can:

  • Drain Funds: Reenter the createOffer function multiple times, causing the contract to think multiple offers are being created and draining more funds than intended.

  • Manipulate State: Reenter the createOffer function and create multiple offers or close offers in a way that benefits them, leading to financial loss for other users.