code4rena-2025-03-starknet-perpetual-h02

[H-02] _execute_transfer wrong order of operations, will first apply diff and then check with applying the diff

보고서

Summary

_execute_transfer 에서 상태 업데이트를 먼저 하고 포지션 건전성을 확인하므로, 실제로는 건전한 상태인 포지션이 건전하지 않다 판단하여 트랜잭션이 취소될 수 있음

Keyword

bug, logic flaw

Vulnerability

• perpetuals/contracts/src/core/core.cairo#L976-L988

_validate_healthy_or_healthier_position 는 apply_diff를 호출하기 전에 먼저 확인해야 한다. 하지만 _execute_transfer 에서는 apply_diff 를 호출한 뒤 _validate_healthy_or_healthier_position 를 호출한다. 이로 인해 포지션의 건전성을 판단할 때, 실제보다 낮은 담보를 가진 것으로 취급된다. 따라서 transfer 는 취소될 수 있다.

_execute_transfer 에서는 apply_diff 를 호출한 뒤 _validate_healthy_or_healthier_position 를 호출한다. apply_diff를 호출하면 요청한 작업이 완료된 포지션이 스토리지에 저장된다. 따라서 get_position_snapshot 는 업데이트된 포지션 정보를 리턴한다. _validate_healthy_or_healthier_position 에서 get_position_snapshot 의 리턴값에 다시 position_diff_sender 를 적용했을 때의 포지션 건전성을 확인하므로, transfer를 2번 했을 때를 기준으로 확인하게 된다. 이는 실제로는 건전한 상태인 포지션이 건전하지 않다 판단하여 트랜잭션을 실패하게 한다.

fn _execute_transfer(
    ref self: ContractState,
    recipient: PositionId,
    position_id: PositionId,
    collateral_id: AssetId,
    amount: u64,
) {
    // Parameters
    let position_diff_sender = PositionDiff {
        collateral_diff: -amount.into(), synthetic_diff: Option::None,
    };
 
    let position_diff_recipient = PositionDiff {
        collateral_diff: amount.into(), synthetic_diff: Option::None,
    };
 
    // Execute transfer
@>  self.positions.apply_diff(:position_id, position_diff: position_diff_sender);
 
    self
        .positions
@>      .apply_diff(position_id: recipient, position_diff: position_diff_recipient);
 
    /// Validations - Fundamentals:
    let position = self.positions.get_position_snapshot(:position_id);
    self
@>      ._validate_healthy_or_healthier_position(
            :position_id, :position, position_diff: position_diff_sender,
        );
}

Impact

건전한 상태인 포지션이 건전하지 않다 판단하여 트랜잭션을 실패하게 한다.

Mitigation

_validate_healthy_or_healthier_position 를 먼저 호출한 뒤 apply_diff를 호출한다.

---

tags: bughunting, starknet, smart contract, cairo, perpetual, severity high, logic flaw