Digital garden

sherlock-2025-04-burve-h04

8 min read

sherlock-2025-04-burve-h04

[H-04] Incorrect tax distribution when adding value single-sided

보고서

Summary

addValueSingle를 호출하면 입금자가 수수료를 지불해야 하고, 지불된 수수료는 기존 유저들에게 분배되어야 한다. 그런데 수수료를 계산, 분배, 상태를 업데이트 하는 로직의 순서가 꼬여 입금자가 낸 수수료를 입금자에게 일부 돌려주고, 기존 유저들은 원래 받아야하는 것보다 적은 리워드를 받는다.

Keyword

logic flaw, staking

Vulnerability

addValueSingle 함수를 호출하면 클로저를 구성하는 모든 토큰이 아니라 한 종류의 토큰만 제공하며 유동성을 제공할 수 있다. 하나의 토큰만 제공하기에 차이가 발생하므로 addValueSingle 에서는 유저에게 수수료를 추가로 받아 기존에 유동성을 제공한 다른 유저들에게 분배한다.

유저는 엔트리포인트 ValueFacet.addValueSingle를 호출하여 진입한다. ValueFacet.addValueSingle에서 Closure.addValueSingle를 호출하여 유저가 제공해야 하는 토큰과 수수료의 양을 계산한다. 수수료는 Closure.addEarnings를 호출하여 해당 클로저의 수입으로 입력한다.

수입은 지분 당 받아야 하는 토큰 양의 체크포인트 형태로 관리되며 유저 지분 * (최신 index - 마지막에 받았던 index) 방식으로 계산해 분배된다. 따라서 이번 수입으로 추가되어야 하는 index를 계산할 때 전체 스테이킹 양으로 나눈다.

// ValueFacet.addValueSingle
function addValueSingle(
    address recipient,
    uint16 _closureId,
    uint128 value,
    uint128 bgtValue,
    address token,
    uint128 maxRequired
) external nonReentrant returns (uint256 requiredBalance) {
    if (value == 0) revert DeMinimisDeposit();
    require(bgtValue <= value, InsufficientValueForBgt(value, bgtValue));
    ClosureId cid = ClosureId.wrap(_closureId);
    Closure storage c = Store.closure(cid); // Validates cid.
    VertexId vid = VertexLib.newId(token); // Validates token.
@>  (uint256 nominalRequired, uint256 nominalTax) = c.addValueSingle(
        value,
        bgtValue,
        vid
    );
    ...
 
@>  c.addEarnings(vid, realTax);
    Store.vertex(vid).deposit(cid, requiredBalance - realTax);
@>  Store.assets().add(recipient, cid, value, bgtValue);
}
 
// Closure.addEarnings
function addEarnings(
    Closure storage self,
    VertexId vid,
    uint256 earnings
) internal {
    uint8 idx = vid.idx();
    // Round protocol take down.
    uint256 protocolAmount = FullMath.mulX128(
        earnings,
        self.protocolTakeX128,
        false
    );
    SimplexLib.protocolTake(idx, protocolAmount);
    uint256 userAmount = earnings - protocolAmount;
    uint256 unspent;
    if (self.bgtValueStaked > 0) {
        // Round BGT take down.
@>      uint256 bgtExAmount = (userAmount * self.bgtValueStaked) /
@>          self.valueStaked; // 지분 당 index 구하기 위해 전체 지분으로 나눔
        uint256 bgtEarned;
        (bgtEarned, unspent) = SimplexLib.bgtExchange(idx, bgtExAmount);
@>      self.bgtPerBgtValueX128 += (bgtEarned << 128) / self.bgtValueStaked;
        userAmount -= bgtExAmount;
    }
    // We total the shares earned and split after to reduce our vault deposits, and
    // we potentially lose one less dust.
    uint256 reserveShares = ReserveLib.deposit(vid, unspent + userAmount);
    if (unspent > 0) {
        // rare
        uint256 unspentShares = (reserveShares * unspent) /
            (userAmount + unspent);
@>      self.unexchangedPerBgtValueX128[idx] +=
            (unspentShares << 128) /
            self.bgtValueStaked; // Must be greater than 0 here.
        reserveShares -= unspentShares;
    }
    // Rest goes to non bgt value.
    self.earningsPerValueX128[idx] +=
        (reserveShares << 128) /
@>      (self.valueStaked - self.bgtValueStaked); // 지분 당 index 구하기 위해 전체 지분으로 나눔
    // Denom is non-zero because all pools start with non-zero non-bgt value.
}

그런데 Closure.addValueSingle에서 유저의 입금액과 수수료를 계산할 때, 이번 입금으로 인한 valueStaked 변동을 업데이트한다. 이로 인해 전체 지분이 업데이트 된 후에 수익 index가 계산된다. 이는 이번 입금자가 지불한 수수료가 이번 입금자에게도 제공됨을 가정한 계산이다.

// Closure.addValueSingle
function addValueSingle(
    Closure storage self,
    uint256 value,
    uint256 bgtValue,
    VertexId vid
) internal returns (uint256 requiredAmount, uint256 tax) {
    ...
 
    // We first calculate what value is effectively "lost" by not adding the tokens.
    // And then we make sure to add that amount of value to the deposit token.
    uint256 fairVBalance = iterSingleValueDiff(self, valIter, true);
    requiredAmount = fairVBalance - self.balances[valIter.vIdx];
    // Now we have the missing value and the currently fair balance for our vertex.
    uint256 finalAmount;
    {
        uint256 veX128 = SimplexLib.getEX128(valIter.vIdx);
        uint256 currentValueX128 = ValueLib.v(
            self.targetX128,
            veX128,
            fairVBalance,
            false
        );
        // To get the required amount.
        finalAmount = ValueLib.x(
            self.targetX128,
            veX128,
            currentValueX128 + valIter.valueSumX128,
            true
        );
    }
    {
        uint256 untaxedRequired = finalAmount - fairVBalance;
        self.setBalance(valIter.vIdx, finalAmount);
        uint256 taxedRequired = UnsafeMath.divRoundingUp(
            untaxedRequired << 128,
            ONEX128 - self.baseFeeX128
        );
@>      tax = taxedRequired - untaxedRequired;
@>      requiredAmount += taxedRequired;
    }
    // This needs to happen after any fee earnings.
@>  self.valueStaked += value;
@>  self.bgtValueStaked += bgtValue;
}

ValueFacet.addValueSingle 의 맨 마지막에 Store.assets().add를 호출하고, 이는 이번 입금자의 입금액을 업데이트한다. 입금액을 업데이트 하기 전 collect를 호출하여 클로저에 발생한 수입을 수집한다. 만약 기존에 예치금이 있던 유저가 ValueFacet.addValueSingle를 호출했다면 자신이 지불한 수수료 일부를 돌려받을 수 있다.

function add(
    AssetBook storage self,
    address recipient,
    ClosureId cid,
    uint256 value,
    uint256 bgtValue
) internal {
@>  collect(self, recipient, cid);
    Asset storage a = self.assets[recipient][cid];
    require(value >= bgtValue, InsufficientValue(value, bgtValue));
    a.value += value;
    a.bgtValue += bgtValue;
}
 
function collect(
    AssetBook storage self,
    address recipient,
    ClosureId cid
) internal {
    (
        uint256[MAX_TOKENS] storage epvX128,
        uint256 bpvX128,
        uint256[MAX_TOKENS] storage unepbvX128
    ) = Store.closure(cid).getCheck();
    Asset storage a = self.assets[recipient][cid];
    uint256 nonBgtValue = a.value - a.bgtValue;
    for (uint8 i = 0; i < MAX_TOKENS; ++i) {
        // Fees
        a.collectedBalances[i] +=
            FullMath.mulX128(
                (epvX128[i] - a.earningsPerValueX128Check[i]),
                nonBgtValue,
                false
            ) +
            FullMath.mulX128(
                (unepbvX128[i] - a.unexchangedPerBgtValueX128Check[i]),
                a.bgtValue,
                false
            );
        a.earningsPerValueX128Check[i] = epvX128[i];
        a.unexchangedPerBgtValueX128Check[i] = unepbvX128[i];
    }
    a.bgtBalance += FullMath.mulX128(
        bpvX128 - a.bgtPerValueX128Check,
        a.bgtValue,
        false
    );
    a.bgtPerValueX128Check = bpvX128;
}

Impact

기존 유저들이 리워드를 실제보다 적게 받는다. 입금자는 자신이 지불한 수수료의 일부를 돌려받을 수 있다.

Mitigation

addEarnings 을 호출하기 전에 self.valueStaked 와 self.bgtValueStaked를 업데이트 하지 않도록 한다. 또는 스냅샷을 찍은 후 이를 이용한다.

Memo

대안으로 제시된 것들이 다 약간 모자란 것 같다. c.addEarnings를 맨 마지막에 호출한다면 여전히 index가 업데이트 되어 이번 입금자가 자신이 입금한 수수료를 일부 돌려받을 수 있다. self.valueStaked 와 self.bgtValueStaked를 업데이트하지 않는 것으로 잘못된 index 업데이트를 방지할 수 있지만, 입금자가 자신의 수수료를 일부 돌려받는 것을 막을 수는 없다. 입금자가 청구했던 인덱스만 업데이트할 수 있는 함수가 있다면 이를 마지막에 호출하는 것으로 방지할 수 있겠다.

tags: bughunting, burve, smart contract, solidity, logic flaw, staking, severity high

Graph View