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

Updating an enumeration in memory rather than in persistent storage results in the changes being applied only locally.

pavankv

Summary

The enum status will be updated in the memory loaction instead of storage which leads to the offer which are turbo status will not be updated as sub offer Listed.

Vulnerability Details

The function listOffer() will be called whenever the createTaker() function is to make the economic activity in the protocol like BUY/SELL. Lets look into the code

/// @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;
}

In the above we can confirm that ,if the offer is turbo then have to change the abort offer status of it. State variable makerInfo.originOffer will be called in the state mapping offerInfoMap to get the originOfferInfo the main issue lay here instead of storage location it updated in memory location leads to update will only in local memory. This issue will create the discrepancy between the turbo offers and non- turbo offers.

Scenario :-

  1. Offer will be created as Turbo

  2. Will called to listed but not updated because of memory location.

  3. User got event emitted consider that Abort status will changed but in reality it will not.

Proof of concept:-

Please check this code which returns the status of whether originOffer abort status changed to the AbortOfferStatus.SubOfferListed or not.

function test_ENUM() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
bool result = preMarktes.getTurboStatus(stock1Addr);
if (result){
console2.log("Changed to Turbo Status"); }
else{
console2.log("Not Changed to Turbo Status");
}
vm.stopPrank();
}
​
// Add this fucntion Premarkets.sol and IPerMarkets.sol to get the turbo status of the stock globally.
​
function getTurboStatus(address stock) external view returns(bool) {
StockInfo storage stockInfo = stockInfoMap[stock];
​
OfferInfo storage offerInfo = offerInfoMap[stockInfo.preOffer];
​
MakerInfo storage makerInfo = makerInfoMap[offerInfo.maker];
​
address originOffer = makerInfo.originOffer;
​
//OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
OfferInfo storage offerInfo2 = offerInfoMap[originOffer];
​
return offerInfo2.abortOfferStatus == AbortOfferStatus.SubOfferListed;
}
​

Output :-

Below we can see the output 1 when the update is in memory loaction

Ran 10 tests for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_ENUM() (gas: 1005105)
Logs:
Not Changed to Turbo Status

Output 2 :-

Below we can see the output 2of the update status when we change the memory location to storage location. In the line :- https://github.com/pavankv241/Tadle-ICP/blob/main/src/core/PreMarkets.sol#L337

Ran 10 tests for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_ENUM() (gas: 1003360)
Logs:
Changed to Turbo Status

Impact

Due Enum updation in the memory location the which are on all created as Turbo which will not updated to the SubOfferListed and leads to against the protocol activity.

Tools Used

Foundry , Manual View

Recommendations

Change the memory location to the storage location in the line

https://github.com/pavankv241/Tadle-ICP/blob/main/src/core/PreMarkets.sol#L337

OfferInfo storage originOfferInfo = offerInfoMap[originOffer]; //@audit change here
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.