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

`collateralTokenAddress` is used instead of `pointsTokenAddress` while adding points tokenBalance to buyer in deliveryPlace::closeBidTaker()

0xaraj

Summary

collateralTokenAddress is used instead of pointsTokenAddress while adding points tokenBalance to buyer in deliveryPlace::closeBidTaker()

Vulnerability Details

When a buyer/user calls deliveryPlace::closeBidTaker() then it calculates the pointsTokenAmount and adds it to msg.sender address. Now the problem is while adding pointsToken, it passes makerInfo.tokenAddress(which is collateralTokenAddress) instead of pointsTokenAddress

function closeBidTaker(address _stock) external {
...
(OfferInfo memory preOfferInfo, MakerInfo memory makerInfo, ,) =
getOfferInfo(stockInfo.preOffer);
​
...
tokenManager.addTokenBalance(
@> TokenBalanceType.PointToken, _msgSender(), makerInfo.tokenAddress, pointTokenAmount
);
...
}

//Here is PoC which shows the pointTokenAmount of buyer is 0, after closing the bid taker

function test_PointsTokenNotUpdated() public {
//User created an ask offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
//User2 bought all 1000 points
vm.prank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
​
//Owner updated the marketPlace
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 0.01 * 1e18, block.timestamp - 1, 3600);
​
//User settled the ask maker
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 1000);
vm.stopPrank();
​
//User2 close the bid taker
vm.prank(user2);
address stock1Addr = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stock1Addr);
​
//Points token of user2 is 0
uint256 pointsTokenAmount =
tokenManager.userTokenBalanceMap(user2, address(mockPointToken), TokenBalanceType.PointToken);
assertEq(pointsTokenAmount, 0);
}

Impact

Collateral token balance of user will be updated instead of points tokenBalance

Tools Used

Manual Review

Recommendations

Use pointsTokenAddress from marketPlaceInfo instead of using makerInfo.tokenAddress(which is collateralToken)

...
- (OfferInfo memory preOfferInfo, MakerInfo memory makerInfo,,) = getOfferInfo(stockInfo.preOffer);
+ (OfferInfo memory preOfferInfo, MakerInfo memory makerInfo, MarketPlaceInfo memory marketPlaceInfo,) =
+ getOfferInfo(stockInfo.preOffer);
...
- tokenManager.addTokenBalance(
- TokenBalanceType.PointToken, _msgSender(), makerInfo.tokenAddress, pointTokenAmount
- );
+ tokenManager.addTokenBalance(
+ 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.