KnightChaser / kcx

A Free-From-Risk crypto trading simulation named KCX(Knightchaser's Cryptocurrency eXchange), built with Svelte + FastAPI + Docker + α
https://kcx.knightchaser.com
4 stars 0 forks source link

Crypto trade API exploitation due to the lack of verification #3

Closed KnightChaser closed 3 months ago

KnightChaser commented 3 months ago

While servicing the KCX after implementing necessary features while transitioning from Svelte to Sveltekit ( Issue #1 ), one of the users(https://github.com/n0paew) found an exploitation technique based on the lack of numerical verification procedures in trade API.

Here's the code that the exploitation triggered

# Buy cryptocurrency (not leverage trading)
@router.post("/api/exchange/trade/buy")
async def buy_crypto(
    request: BuyCryptoSchema,
    db: Session = Depends(get_sqlite3_db),
    redis_client = Depends(get_redis_db),
    current_user: User = Depends(get_current_user)
) -> JSONResponse:
    try:
        current_price: float = await get_current_crypto_price(f"KRW-{request.market_code}", redis_client)
    except Exception as exception:
        raise HTTPException(status_code=500, detail=f"Failed to get the current price of the cryptocurrency: {str(exception)}")

    # Get the user's balance
    username: str = current_user["username"]
    user_id: int = get_user_id_by_username(username, db)
    user_balance: Balance = db.query(Balance).filter(Balance.user_id == user_id).first()
    if not user_balance:
        raise HTTPException(status_code=404, detail="User balance not found")

    # Calculate the price information
    fee_rate: float = 0.0005 # 0.05% fee
    total_buy_price: float = current_price * request.amount * (1 + fee_rate)
    if user_balance.KRW < total_buy_price:
        raise HTTPException(status_code=400, detail="Insufficient balance")

    crypto_amount_attr = request.market_code
    crypto_avg_price_attr = f"{request.market_code}_average_unit_price"

    current_num_assets = getattr(user_balance, crypto_amount_attr, 0)
    current_avg_price = getattr(user_balance, crypto_avg_price_attr, 0)

    # Calculate new number of assets and new average price
    # (Calculating the entry price considering the average price of the existing assets)
    new_num_assets = current_num_assets + request.amount
    new_avg_price = ((current_avg_price * current_num_assets) + (current_price * request.amount)) / new_num_assets

    # Update the user's balance
    # (The value will be applied to the database)
    user_balance.KRW -= total_buy_price
    setattr(user_balance, crypto_amount_attr, new_num_assets)
    setattr(user_balance, crypto_avg_price_attr, new_avg_price)

    db.commit()
    db.refresh(user_balance)

    # Save the trade history
    new_trade_history = TradeHistory(user_id=user_id,
                                     currency=request.market_code,
                                     amount=request.amount,
                                     price=current_price,
                                     transaction_type="buy",
                                     leverage_ratio=1)
    db.add(new_trade_history)
    db.commit()

    # Add the total transaction amount to the statistics
    # (The value will be applied to the database)
    statistics = db.query(Statistics).first()
    if not statistics:
        statistics = Statistics(total_transaction_amount=total_buy_price)
        db.add(statistics)
    else:
        statistics.total_transaction_amount += total_buy_price
    db.commit()

    # Return the response with data
    response_data: dict = {
        "message": "Successfully bought the cryptocurrency",
        "currency": request.market_code,
        "amount": request.amount,
        "price": current_price,
        "total_price": total_buy_price,
        "fee_rate": fee_rate,
        "fee": total_buy_price * fee_rate
    }
    return JSONResponse(content=response_data, status_code=200)

While saving the crypto transactions,

    # Save the trade history
    new_trade_history = TradeHistory(user_id=user_id,
                                     currency=request.market_code,
                                     amount=request.amount,
                                     price=current_price,
                                     transaction_type="buy",
                                     leverage_ratio=1)
    db.add(new_trade_history)
    db.commit()

There is no verification procedure for the numerical values. At least, they can't be less than zero. Thus, we need two additional verification procedures for both .../buy and .../sell API endpoints. (Frontend verification isn't enough because the payload can be easily altered.)

Here's the PoC provided by @n0paew

KnightChaser commented 3 months ago

This patch will be applied to #2 because the impact of this issue is substantial. Currently patched, so close the issue. The issue may be reopened if needed.