Bellum Core - Bellum Exchange

In BellumFactory contract, the createToken() function combined with the buy() functionality exposes a potential reentrancy attack vector that could allow an attacker to manipulate conditions or drain AVAX from the contract.

The vulnerability exists because:

1. Contract holds AVAX balances during token creation and launch

2. External call pattern in buy() -> buy() -> launchToken() flow that returns excess AVAX

3. No reentrancy guard on createToken()

4. State changes occur after external calls

Attack Scenario:

1. Attacker creates token with carefully calculated parameters

2. During buy() execution, reaches final bin triggering _launchToken()

3. Before state updates, excess AVAX is returned via sendValue()

4. Attacker contract receives AVAX and reenters

5. Original execution continues with a compromised state

6. Attacker potentially extracts additional AVAX or manipulates launch conditions

   
   

While currently mitigated by token pricing mechanics, changes to pricing parameters or discovery of specific attack parameters could make this exploitable in the future.

Add ReentrancyGuard to BellumFactory::createToken():

function createToken(...) external payable nonReentrant returns (address) {

SOLVED: The suggested mitigation was implemented by the Bellum Exchange team.

The BellumFactory contract approves an unlimited amount (i.e.: type(uint256).max) of newly created tokens to external DEX routers (TJ_ROUTER or PHARAOH_ROUTER). While this is done for convenience, it creates a security risk:

If either router contract is compromised through:

• Implementation vulnerabilities

• Upgradeable proxy attacks

• Admin key compromise

• Future bugs

The attacker could:

• Drain all tokens held by the BellumFactory

• Manipulate token launches

• Extract value from the protocol

• Impact multiple tokens simultaneously

Replace infinite approvals with exact amount approvals during token launch.

SOLVED: The suggested mitigation was implemented by the Bellum Exchange team.

The BellumNursery.emergencyWithdrawal function incorrectly includes the waitForWithdrawal modifier, which enforces a 6-epoch waiting period before withdrawals. This fundamentally contradicts the purpose of an emergency withdrawal mechanism:

Current Implementation Issues:

• Users must wait 6 epochs (36 hours) even in emergencies

• Cannot quickly respond to:

  • Smart contract vulnerabilities

  • Market crashes

  • Protocol exploits

  • Network issues

Emergency Function Requirements:

• Should provide immediate exit route

• Critical for user fund safety

• Standard DeFi safety mechanism

• Expected behavior by users

Remove timelock from emergency withdrawal:

function emergencyWithdrawal() external nonReentrant

SOLVED: The function was removed from the contract.

The BellumFactory contract inherits from OpenZeppelin's Ownable but doesn't implement a two-step ownership transfer process. With the current single-step transfer, if the owner accidentally transfers ownership to an incorrect address, all owner-privileged functions will be permanently lost. This is particularly risky given the contract's critical fee management and pause functionality.

Implement a two-step ownership transfer pattern:

1. First transaction: Current owner indicates intended new owner

2. Second transaction: New owner accepts ownership

This can be achieved by using OpenZeppelin's Ownable2Step instead of Ownable.

SOLVED: The suggested mitigation was implemented by the Bellum Exchange team.

In BellumFactory contract, the token launch function violates the Checks-Effects-Interactions (CEI) pattern and introduces centralization risks through fee distribution:

1. CEI Pattern Violation:

• External call (sendValue) is made before important state changes

• Token ownership is renounced but contract state isn't fully updated

• LP creation happens after potentially failed fee transfer

2. Centralization Risks:

• Single feeReceiver address can block token launches

• No fallback mechanism if fee transfer fails

• Could be used maliciously to prevent specific tokens from launching

Implement pull pattern for fees which sends the fees to feeReceiver:

// Add state tracking
uint256 public pendingFees;

function _launchToken(address token) internal {
    // Calculate fee
    uint256 fee = avaxToLP * migrationFee / BASIS;
    
    // Update state first
    if (fee > 0) {
        pendingFees += fee;
    }
    
    // Complete token launch
    IOwnable(token).renounceOwnership();
    // ... rest of launch logic ...
    
    emit TokenLaunched(token, fee);
}

// Separate fee collection function
function collectFees() external{
    uint256 amount = pendingFees;
    require(amount > 0, "No fees to collect");
    
    pendingFees = 0;
    (bool success, ) = feeReceiver.call{value: amount}("");
    require(success, "Fee transfer failed");
    
    emit FeesCollected(feeReceiver, amount);
}

SOLVED: All the instances, wherein the fees were sent to feeReceiver, are now replaced with a pull pattern:

-   feeReceiver.sendValue(createFee_);
+   pendingFees += createFee_;

In BellumFactory contract, the constructor accepts critical addresses (owner_, tjRouter_, pharaohRouter_, incentivizer_) without validating that they are not the zero address (0x0). If any of these parameters are accidentally set to the zero address, it could lead to locked functionality or loss of funds since:

• Zero address owner would make the contract permanently ownerless

• Invalid router addresses would break core trading functionality

• Zero address incentivizer would lead to lost token incentives

Add zero address validation checks in the constructor:

require(owner_ != address(0), "Zero address not allowed for owner");
require(tjRouter_ != address(0), "Zero address not allowed for TJ router");
require(pharaohRouter_ != address(0), "Zero address not allowed for Pharaoh router");
require(incentivizer_ != address(0), "Zero address not allowed for incentivizer");

SOLVED: The suggested mitigation was implemented by the Bellum Exchange team.

In BellumFactory contract, the critical protocol parameters like tradingFee, migrationFee, ETHER, BASIS, BIN_WIDTH, COEF, MIN_IN, MIN_OUT, incentivizerFee, createFee, and percentPerEpoch are hardcoded in the constructor.

Move constant values to constructor parameters. The other way is to define truly constant values (like BASIS = 10_000) as immutable state variables outside the constructor.

SOLVED: Constant values were defined for the given variables:

 uint256 private constant ETHER = 1 ether;
 uint256 private constant BIN_WIDTH = 2000;
 uint256 private constant BASIS = 10000;
 uint256 private constant COEF = 100;
 uint256 private constant MIN_IN = 0.1 ether;
 uint256 private constant MIN_OUT = 0.02 ether;

In BellumFactory contract, none of the admin functions that modify critical protocol parameters emit events. This includes changes to:

• Fee receiver address

• Trading fees

• Pause state

• Migration fee

• Epoch percentage

• Incentivizer fee

• Creation fee

Without events, it becomes difficult to:

• Track historical changes to important parameters

• Build monitoring systems

• Audit administrative actions

Add events for each state change:

// Declare events
event FeeReceiverChanged(address indexed oldReceiver, address indexed newReceiver);
event TradingFeeChanged(uint256 oldFee, uint256 newFee); 
event PauseStateChanged(bool isPaused);
event MigrationFeeChanged(uint256 oldFee, uint256 newFee);
event PercentPerEpochChanged(uint256 oldPercent, uint256 newPercent);
event IncentivizerFeeChanged(uint256 oldFee, uint256 newFee);
event CreateFeeChanged(uint256 oldFee, uint256 newFee);

// Emit in functions
function changeFeeReceiver(address payable feeReceiver_) external onlyOwner {
   address oldReceiver = feeReceiver;
   feeReceiver = feeReceiver_;
   emit FeeReceiverChanged(oldReceiver, feeReceiver_);
}

// Add similar event emissions to other state-changing functions

ACKNOWLEDGED: The Bellum Exchange team acknowledged this finding.