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

Ask-Takers Incorrectly Charged ERC20 Tokens Instead of Points

0xjoyboy03

Summary

Ask-takers will charge erc20 token to get erc20 token

Vulnerability Details

when ask-takers want to sell their points by calling the PreMarkets.createTaker function, they will be charged to transfer an erc20 token to sell their points. but this is unintended because the PreMarkets contract should charge the ask-takers points not the erc20 token. therefore, in the process of selling points, ask-takers never exit their points to sell it

function _depositTokenWhenCreateTaker(
uint256 platformFee,
uint256 depositAmount,
uint256 tradeTax,
MakerInfo storage makerInfo,
OfferInfo storage offerInfo,
ITokenManager tokenManager
) internal {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Ceil
);
​
transferAmount = transferAmount + platformFee + tradeTax;
​
tokenManager.tillIn{value: msg.value}(
_msgSender(),
>>> makerInfo.tokenAddress,
transferAmount,
false
);
}

when the ask-takers call the createTaker function, the _depositTokenWhenCreateTaker func will be called to deposit the points to the capitalPool contract. however, they will force you to transfer the erc20 token which is determined by the maker in the amount of transferAmount and not the point token. additionally, the _isPointToken param from the TokenManager::tillIn func is false which must be true cause the ask-takers want to sell points and receive an erc20 token. the contract must take their points to give it to bid-makers but instead, ask-takers are forced to transfer their desired output which is unintended
in this scenario, bid-makers won't get their desired points, and this leads to malfunctioning of the protocol due to unhandling and accounting for the points properly by the PreMarkets contract

This test illustrates the scenario by adding it to PreMarkets.t.sol:

function test_AskTakersNeverExitTheirPoints() public {
​
address _offer1 = GenerateAddress.generateOfferAddress(0);
​
uint256 user2PointsBalBef = mockPointToken.balanceOf(user2);
uint256 user2USDCBalBef = mockUSDCToken.balanceOf(user2);
​
vm.prank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
10000,
300,
OfferType.Bid,
OfferSettleType.Protected
));
​
vm.prank(user2);
preMarktes.createTaker(_offer1, 1000);
​
uint256 user2PointsBalAf = mockPointToken.balanceOf(user2);
uint256 user2USDCBalAf = mockUSDCToken.balanceOf(user2);
​
assertEq(user2PointsBalAf, user2PointsBalBef);
assertTrue(user2USDCBalBef > user2USDCBalAf);
}

Impact

Ask-takers are forced to transfer their desired output and Bid-makers won't get their points, leading to disruption of the main functionality of the protocol

Tools Used

manual review

Recommendations

change the _depositTokenWhenCreateTaker to this:

function _depositTokenWhenCreateTaker(
uint256 platformFee,
uint256 depositAmount,
uint256 tradeTax,
MakerInfo storage makerInfo,
OfferInfo storage offerInfo,
ITokenManager tokenManager
) internal {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Ceil
);
​
transferAmount = transferAmount + platformFee + tradeTax;
​
if(offerInfo.offerType == OfferType.Bid) {
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
​
tokenManager.tillIn{value: msg.value}(
_msgSender(),
marketPlaceInfo.tokenAddress,
depositAmount,
true
);
​
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
platformFee + tradeTax,
false
);
} else {
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
);
}
}

Additionally, in this mitigation the marketPlaceInfo.tokenAddress is not assigned yet so you should set the point token address when creating a market otherwise, it won't work cause

function createMarketPlace(
string calldata _marketPlaceName,
bool _fixedratio,
+ address _tokenAddress
) external onlyOwner {
address marketPlace = GenerateAddress.generateMarketPlaceAddress(
_marketPlaceName
);
MarketPlaceInfo storage marketPlaceInfo = marketPlaceInfoMap[
marketPlace
];
​
if (marketPlaceInfo.status != MarketPlaceStatus.UnInitialized) {
revert MarketPlaceAlreadyInitialized();
}
​
marketPlaceInfo.status = MarketPlaceStatus.Online;
marketPlaceInfo.fixedratio = _fixedratio;
+ marketPlaceInfo.tokenAddress = _tokenAddress;
​
emit CreateMarketPlaceInfo(_marketPlaceName, marketPlace, _fixedratio);
}
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.