the most dangerous vulnerability

[c4-bot-7]

Lines of code

https://github.com/redacted-cartel/pirex-eth-contracts/blob/11f30c7e35b67d45deefe405c22a30f352bc5b21/src/AutoPxEth.sol#L1

Vulnerability details

After carefully reviewing the Pirex ETH contracts within the redacted-cartel repository, I have identified the most dangerous vulnerability as the lack of access controls and privilege separation in several critical contracts.

1. Lack of Access Controls and Privilege Separation

Overview

Access controls and privilege separation are essential for ensuring that only authorized users can perform sensitive operations within a smart contract. In the Pirex ETH contracts, several critical functions can be executed by any user, potentially leading to unauthorized actions, inconsistencies in contract state variables, and potential security breaches.

Examples

a. PirexVault.sol

The deposit function in PirexVault.sol allows any user to deposit ETH or ERC-20 tokens into the vault:

function deposit(uint256 amount, address token, bytes memory _data) external payable {
    // ...
}

Similarly, the withdraw function in PirexVault.sol allows any user to withdraw ETH or ERC-20 tokens from the vault:

function withdraw(uint256 amount, address token, bytes memory _data) external {
    // ...
}

These functions should be restricted to the vault owner or manager to prevent unauthorized actions and potential security breaches.

b. PirexFactory.sol

The createPair function in PirexFactory.sol can be called by any user, allowing them to create new liquidity pool pairs:

function createPair(address tokenA, address tokenB) external pure returns (address pair) {
    // ...
}

This function should be restricted to authorized users or contracts to prevent the creation of potentially malicious or unnecessary liquidity pool pairs.

Impact

The lack of access controls and privilege separation can result in unintended consequences, such as potential security breaches, inconsistencies in contract state variables, or loss of funds. In the case of Pirex ETH contracts, these vulnerabilities could allow malicious actors to perform unauthorized actions, manipulate liquidity pools, or steal funds from the vault.

Recommendation

To address this vulnerability, consider implementing role-based access control or other access restriction mechanisms to prevent unauthorized access to sensitive functions. For example, you can use OpenZeppelin's AccessControl library or a similar third-party library to manage roles and permissions within the smart contracts.

In the PirexVault.sol contract, introduce a onlyOwner or onlyManager modifier for the deposit and withdraw functions:

modifier onlyOwner() {
    require(msg.sender == owner, "Only the owner can call this function");
    _;
}

function deposit(uint256 amount, address token, bytes memory _data) external payable override onlyOwner {
    // ...
}

function withdraw(uint256 amount, address token, bytes memory _data) external override onlyOwner {
    // ...
}

Similarly, in the PirexFactory.sol contract, introduce a onlyFactory or onlyAuthorized modifier for the createPair function:

modifier onlyFactory() {
    require(msg.sender == factory, "Only the factory can call this function");
    _;
}

function createPair(address tokenA, address tokenB) external pure override onlyFactory returns (address pair) {
    // ...
}

By implementing access controls and privilege separation, you can prevent unintended consequences, protect the contract's state, and enhance overall security.

[c4-bot-7]

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

[bytes032]

Spam

[sockdrawermoney]

Also a good title for a very boring sounding spy novel