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

Due to incorrect implementation, the core functions of the protocol are damaged.

0xhunter

Summary

In listOffer(), the core function of the protocol are damaged because mapping variable is set to memory

Vulnerability Details

User calls listOffer() to create a new offer, typically for selling points. This is done by the owner of the stock (_stock parameter).

function listOffer()
...
​
/// @dev change abort offer status when offer settle type is turbo
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
@ OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
@ originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}
​
...

As you can see, originOffer was defined to memory, as a result, even though originOfferInfo.abortOfferStatus was set to AbortOfferStatus.SubOfferListed, offerInfoMap[originOffer].abortOfferStatus was not changed and maintained original value. so the core function of the protocol is damaged. For example let's look at abortAskOffer().

function abortAskOffer(address _stock, address _offer) external {
...
​
if (offerInfo.abortOfferStatus != AbortOfferStatus.Initialized) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Initialized,
offerInfo.abortOfferStatus
);
}
​
...
}

As you can see above, even though abortAskOffer() should be reverted, it is not being reverted.

Proof of Code

function test_turbo_subofferlisted() public {
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
preMarktes.OfferInfo memory initialOfferInfo = preMarktes.offerInfoMap(GenerateAddress.generateOfferAddress(0));
assertEq(uint(initialOfferInfo.abortOfferStatus), uint(OfferContract.AbortOfferStatus.Initialized));
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
​
// Check status after calling listOffer
preMarktes.OfferInfo memory updatedOfferInfo = preMarktes.offerInfoMap(GenerateAddress.generateOfferAddress(0));
assertEq(uint(updatedOfferInfo.abortOfferStatus), uint(OfferContract.AbortOfferStatus.Initialized)); // Should remain Initialized
vm.stopPrank();
}

Impact

The core function of the protocol are damaged

Tools Used

Mannual Review

Recommendations

listOffer() is modified as follow.

function listOffer()
...
​
/// @dev change abort offer status when offer settle type is turbo
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
--- OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
+++ OfferInfo storage originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}
​
...
Updates

Lead Judging Commences

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

finding-PreMarkets-listOffer-originIOfferInfo-storage-memory

Valid high severity, because the `abortOfferStatus` of the offer is not updated and persist through `storage` when listing an offer for turbo mode within the `offerInfoMap` mapping, it allows premature abortion given the `abortOfferStatus` defaults to `Initialized`, allowing the bypass of this [check](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L552-L557) here and allow complete refund of initial collateral + stealing of trade tax which can potentially be gamed for profits using multiple addresses

Support

FAQs

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