Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: low
Invalid

Contract interfacing with the PerMarketsStorage upgradable contract `offerInfoMap;` is exposed to cross function reentracies, that can drain the protocol and cause data manipulation.

Summary

The PerMarketsStorage upgradable contrct serves as the storage of PerMarkets. But it does not have access modifiers to enforce access control to ensure only authorized entities can modify or read sensitive data. This exposes any contract interfacing with it to cross function reentrancies:
PreMarktes.abortAskOffer,
PreMarktes.abortBidTaker,
PreMarktes.closeOffer,
PreMarktes.createOffer,
PreMarktes.createTaker,
PreMarktes.getOfferInfo,
PreMarktes.listOffer,
PerMarketsStorage.offerInfoMap,
PreMarktes.relistOffer,
PreMarktes.settleAskTaker,
PreMarktes.settledAskOffer,
PreMarktes.updateOfferStatus

Vulnerability Details

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.13;
import {UpgradeableStorage} from "./UpgradeableStorage.sol";
import {OfferStatus} from "./OfferStatus.sol";
import {OfferInfo, StockInfo, MakerInfo} from "../interfaces/IPerMarkets.sol";
/**
* @title PerMarketsStorage
* @notice This contrct serves as the storage of PerMarkets
* @notice The top 50 storage slots are used for upgradeable storage.
* @notice The 50th to 150th storage slots are used for PerMarkets.
*/
contract PerMarketsStorage is UpgradeableStorage {
/// @dev the last offer id. increment by 1
/// @notice the storage slot is 50
uint256 public offerId;
/// @dev offer account => offer info.
/// @notice the storage slot is 51
@> mapping(address => OfferInfo) public offerInfoMap;//@audit can cause cross function reentrancies
/// @dev stock account => stock info.
/// @notice the storage slot is 52
@> mapping(address => StockInfo) public stockInfoMap; //@audit can cause cross function reentrancies
/// @dev maker account => maker info.
/// @notice the storage slot is 53
@> mapping(address => MakerInfo) public makerInfoMap; //@audit can cause cross function reentrancies
/// @dev empty reserved space is put in place to allow future versions to add new
/// variables without shifting down storage in the inheritance chain.
/// See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
/// start from slot 54, end at slot 149
uint256[96] private __gap;
}

Impact

cross function reentrancies would lead to drainage of funds or data manipulation

Tools Used

manual review

Recommendations

for PerMarketsStorage UpgradeableStorage contract
i. Follow OpenZeppelin's recommended practices for upgradeable contracts.

For any contract interfacing with the PerMarketsStorage UpgradeableStorage contract
ii. You can implement a reentrancy guard or
iii. Implement Check-Effects-Interactions (CEI) Pattern

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
Assigned finding tags:

[invalid] finding-PreMarkets-reentrancy

Invalid, all [vague generalities](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#vague-generalities) talking about possible reentrancies 11and afaik, reentrancy is not possible and not proven.

Support

FAQs

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