code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

Only admin can call peg functions #2207

Closed code423n4 closed 10 months ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1054

Vulnerability details

Impact

Only admin (multisig) can call peg functions. This might delay re-pegs.

Proof of Concept

Both upperDepeg and lowerDepeg are meant to be called by any EOA or whitelisted contracts, but due to the onlyRole modifier, only the admin can currently call these functions:

   * @notice Lets users mint DpxEth at a 1:1 ratio when DpxEth pegs above 1.01 of the ETH token
   * @param  _amount The amount of DpxEth to mint
   * @param minOut The minimum amount out
   **/
  function upperDepeg(
    uint256 _amount,
    uint256 minOut
  ) external onlyRole(DEFAULT_ADMIN_ROLE) returns (uint256 wethReceived) {
    _isEligibleSender();

...

  // modifier is eligible sender modifier
  function _isEligibleSender() internal view {
    // the below condition checks whether the caller is a contract or not
    if (msg.sender != tx.origin)
      require(whitelistedContracts[msg.sender], "Contract must be whitelisted");
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Remove the onlyRole modifier from the re-pegging functions.

Assessed type

Other

bytes032 commented 1 year ago

Peg Stability Module (PSM)

The peg stability module is part of the [Core Contract](https://www.notion.so/rDPX-V2-RI-b45b5b402af54bcab758d62fb7c69cb4?pvs=21) that ensures that the peg of dpxETH to ETH on the curve pool is maintained. It is entirely controlled via the [Core Contract](https://www.notion.so/rDPX-V2-RI-b45b5b402af54bcab758d62fb7c69cb4?pvs=21) managers and It encompasses 3 functions:

https://dopex.notion.site/rDPX-V2-RI-b45b5b402af54bcab758d62fb7c69cb4

c4-pre-sort commented 1 year ago

bytes032 marked the issue as low quality report

GalloDaSballo commented 11 months ago

I think otherwise creates further risks as you can manipulate the reserves

c4-judge commented 10 months ago

GalloDaSballo marked the issue as unsatisfactory: Invalid