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

PerMarketsStorage.offerInfoMap can be used in cross function reentrancies:

emma77

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 This exposes any contract interfacing with it to cross function reentrancies:

data.PerMarketsStorage.offerInfoMap can be used in 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 { //@audit follow openzepeppelin upgradeable storage
/// @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 site reentrancy
​
/// @dev stock account => stock info.
/// @notice the storage slot is 52
mapping(address => StockInfo) public stockInfoMap; //@audit can cause cross site reentrancy
​
/// @dev maker account => maker info.
/// @notice the storage slot is 53
mapping(address => MakerInfo) public makerInfoMap; //@audit can cause cross site reentrancy
​
/// @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

Exposes the protocol to reentrancy attack, making the protocol to lose assets

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.