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

`settleAskMaker` function needs to call `tokenManager.addTokenBalance` function for `TokenBalanceType.PointToken`-matched stock's authority-`marketPlaceInfo.tokenAddress`-`settledPointTokenAmount` combination if `settledPointTokenAmount > 0` is true

rbserver

Summary

When calling the settleAskMaker function, because the tokenManager.addTokenBalance function is not called for the TokenBalanceType.PointToken - matched stock's authority - marketPlaceInfo.tokenAddress - settledPointTokenAmount combination if settledPointTokenAmount > 0 is true, the matched stock's authority's claimable point token balance for TokenBalanceType.PointToken is not increased by such settledPointTokenAmount in the token manager though it should be. In this case, such matched stock's authority would not able to claim such settledPointTokenAmount of the point token that is entitled to him.

Vulnerability Details

When calling the following settleAskMaker function, tokenManager.tillIn(_msgSender(), marketPlaceInfo.tokenAddress, settledPointTokenAmount, true) would be executed if settledPointTokenAmount > 0 is true. However, the tokenManager.addTokenBalance function is not called for the TokenBalanceType.PointToken - matched stock's authority - marketPlaceInfo.tokenAddress - settledPointTokenAmount combination in this case; hence, the userTokenBalanceMap for the matched stock's authority - marketPlaceInfo.tokenAddress - TokenBalanceType.PointToken combination is not increased by settledPointTokenAmount when it should be. In other words, after _msgSender() sends settledPointTokenAmount of the point token to the capital pool through the tokenManager.tillIn function call, the matched stock's authority's claimable point token balance for TokenBalanceType.PointToken is not increased by such settledPointTokenAmount in the token manager though it should be.

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/DeliveryPlace.sol#L222-L325

function settleAskMaker(address _offer, uint256 _settledPoints) external {
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
MarketPlaceStatus status
) = getOfferInfo(_offer);
​
if (_settledPoints > offerInfo.usedPoints) {
revert InvalidPoints();
}
​
if (marketPlaceInfo.fixedratio) {
revert FixedRatioUnsupported();
}
​
if (offerInfo.offerType == OfferType.Bid) {
revert InvalidOfferType(OfferType.Ask, OfferType.Bid);
}
​
if (
offerInfo.offerStatus != OfferStatus.Virgin &&
offerInfo.offerStatus != OfferStatus.Canceled
) {
revert InvalidOfferStatus();
}
​
if (status == MarketPlaceStatus.AskSettling) {
if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
​
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint *
_settledPoints;
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
@> if (settledPointTokenAmount > 0) {
@> tokenManager.tillIn(
@> _msgSender(),
@> marketPlaceInfo.tokenAddress,
@> settledPointTokenAmount,
@> true
@> );
@> }
​
uint256 makerRefundAmount;
if (_settledPoints == offerInfo.usedPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else {
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
​
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
makerRefundAmount
);
}
​
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
perMarkets.settledAskOffer(
_offer,
_settledPoints,
settledPointTokenAmount
);
...
}

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/TokenManager.sol#L113-L129

function addTokenBalance(
TokenBalanceType _tokenBalanceType,
address _accountAddress,
address _tokenAddress,
uint256 _amount
) external onlyRelatedContracts(tadleFactory, _msgSender()) {
userTokenBalanceMap[_accountAddress][_tokenAddress][
_tokenBalanceType
] += _amount;
...
}

Impact

Since the matched stock's authority's claimable point token balance for TokenBalanceType.PointToken is not increased by settledPointTokenAmount in the token manager though it should be, such matched stock's authority would not be able to claim such settledPointTokenAmount of the point token that is entitled to him.

Tools Used

Manual Review

Recommended Mitigation

The settleAskMaker function can be updated to call tokenManager.addTokenBalance function for the TokenBalanceType.PointToken - matched stock's authority - marketPlaceInfo.tokenAddress - settledPointTokenAmount combination after tokenManager.tillIn(_msgSender(), marketPlaceInfo.tokenAddress, settledPointTokenAmount, true) is executed in the settledPointTokenAmount > 0 if block.

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

Appeal created

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

finding-DeliveryPlace-settleAskMaker-addTokenBalance-wrong-TokenBalanceType

Valid low severity, while the token type inputted is wrong, userTokenBalanceMap is still incremented appropriately, so users can still withdraw their funds. So this would technically only affect accounting and public view functions.

Support

FAQs

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