The distributeFees function does not have explicit access controls, any address can trigger it.

[c4-bot-8]

Lines of code

https://github.com/redacted-cartel/pirex-eth-contracts/blob/master/src/PirexFees.sol#L78

Vulnerability details

The distributeFees function does not have explicit access controls, meaning any address can trigger it as long as it provides valid parameters.

To make the distributeFees function in the PirexFees contract properly controlled, we can introduce an access control mechanism. A simple and effective approach is to utilize role-based access control, specifically by using a FEE_DISTRIBUTOR_ROLE. This role can be managed by the contract owner (or another designated admin role), and only addresses granted this role should be allowed to execute the distributeFees function.

Original function:

function distributeFees( address from, address token, uint256 amount ) external { emit DistributeFees(token, amount);

    ERC20 t = ERC20(token);

    // Favoring push over pull to reduce accounting complexity for different tokens
    t.safeTransferFrom(from, recipient, amount);
}

Here is how you could modify the PirexFees contract to include this security enhancement:

Updated Contract with Access Control:

// SPDX-License-Identifier: MIT pragma solidity 0.8.19;

import {Ownable2Step} from "openzeppelin-contracts/contracts/access/Ownable2Step.sol"; import {ERC20} from "solmate/tokens/ERC20.sol"; import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; import {AccessControl} from "openzeppelin-contracts/contracts/access/AccessControl.sol"; import {DataTypes} from "./libraries/DataTypes.sol"; import {Errors} from "./libraries/Errors.sol";

/**

• @title PirexFees
• @notice Handling protocol fees distributions with controlled access

• @dev This contract manages the distribution of protocol fees to assigned recipient with role-based access controls. */ contract PirexFees is Ownable2Step, AccessControl { using SafeTransferLib for ERC20;

  // Define a role for distributors bytes32 public constant FEE_DISTRIBUTOR_ROLE = keccak256("FEE_DISTRIBUTOR_ROLE");

  address public recipient;

  event SetRecipient(address recipient); event DistributeFees(address token, uint256 amount);

  constructor(address _recipient) { if (_recipient == address(0)) revert Errors.ZeroAddress(); recipient = _recipient;

  // Setup the owner as the default admin role which can manage other roles
  _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);

  }

  function setRecipient(address _recipient) external onlyOwner { if (_recipient == address(0)) revert Errors.ZeroAddress(); emit SetRecipient(_recipient); recipient = _recipient; }

  /**

  • @notice Distribute fees from a specific source.
  • @param from Address where the fees are collected from.
  • @param token Token address which is being distributed.
  • @param amount Amount of the token to distribute. */ function distributeFees(address from, address token, uint256 amount) external onlyRole(FEE_DISTRIBUTOR_ROLE) { emit DistributeFees(token, amount); ERC20 t = ERC20(token); t.safeTransferFrom(from, recipient, amount); } }

Key Changes Explained

1. Import AccessControl: Import AccessControl from OpenZeppelin to manage role-based access controls effectively.
2. Define FEE_DISTRIBUTOR_ROLE: A role specifically for addresses that can distribute fees. This ensures that only authorized entities can perform fee distributions.
3. Role Setup in Constructor: The owner is set up with the default admin role, which allows the owner to manage other roles. This is useful for granting or revoking the FEE_DISTRIBUTOR_ROLE to other addresses.
4. Modify distributeFees Function: The function is modified to include onlyRole(FEE_DISTRIBUTOR_ROLE), which checks that the caller has the necessary role to execute the function. This prevents unauthorized access and ensures that fee distribution is controlled.

These changes introduce a robust control mechanism to manage who can distribute fees, aligning with best practices in contract security, particularly in the context of decentralized finance where managing financial transactions securely is crucial.

[c4-bot-9]

Discord id(s) for hunter(s): [object Object]

[MiloTruck]

Invalid. distributeFees() can be called by anyone as the tokens will be sent to recipient regardless.