it does not perform any checks on the burner accounts' balance before attempting to burn the specified amount of pxETH.

[c4-bot-3]

Lines of code

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

Vulnerability details

I found a potential vulnerability related to the _updateBuffer function. This function updates the buffer by burning a specified amount of pxETH from an array of burner accounts. However, it does not perform any checks on the burner accounts' balance before attempting to burn the specified amount of pxETH.

If a malicious user passes an account with an insufficient balance, the _burnPxEth function will fail, and an exception will be thrown. However, the smart contract does not have any error handling in place, resulting in the contract execution stopping at this point. This can be potentially exploited by an attacker to cause the contract to halt its execution.

To mitigate this vulnerability, I recommend adding balance checks before attempting to burn pxETH in the _updateBuffer function. You can check the balance of each burner account using the balanceOf function provided by the ERC20 token contract and ensure there's enough balance to perform the burn. If the balance is insufficient, you can skip that account and proceed with the next one, or you can revert the transaction with an error message indicating the issue.

Additionally, it might be a good practice to include re-entrancy protection mechanisms for the _updateBuffer function to prevent potential attacks that exploit re-entrancy vulnerabilities. Re-entrancy protection can be added using a mutex or re-entrancy guard, such as the one used in the provided smart contract (ReentrancyGuard). Thank you for the input. I understand the vulnerability and the suggested mitigation. I will add balance checks before attempting to burn pxETH in the _updateBuffer function and include re-entrancy protection using the ReentrancyGuard mechanism.

Here's an updated version of the _updateBuffer function with the suggested changes:

function _updateBuffer(
    uint256 _amount,
    DataTypes.BurnerAccount[] calldata _burnerAccounts
) private ReentrancyGuard {
    if (buffer < _amount) {
        revert Errors.NotEnoughBuffer();
    }
    uint256 _len = _burnerAccounts.length;
    uint256 _sum;

    for (uint256 _i; _i < _len; ) {
        if (!burnerAccounts[_burnerAccounts[_i].account]) {
            revert Errors.AccountNotApproved();
        }

        if (pxEth.balanceOf(_burnerAccounts[_i].account) < _burnerAccounts[_i].amount) {
            revert Errors.InsufficientBalance();
        }

        _sum += _burnerAccounts[_i].amount;

        _burnPxEth(_burnerAccounts[_i].account, _burnerAccounts[_i].amount);

        unchecked {
            ++_i;
        }
    }

    assert(_sum == _amount);
    buffer -= _amount;
}

With the added balance checks and re-entrancy protection, the _updateBuffer function should now be more secure and robust.

[c4-bot-4]

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