Code Review: shop.py (쇼핑몰 주문 처리 모듈)

검토일: 2026-10-16 · 대상: shop.py (85줄, sqlite3 기반) 검증 방법: 코드 정독 + 임시 DB를 만들어 아래 공격·버그 시나리오를 실제로 실행해 재현 확인

Summary

기능 흐름은 단순하고 대부분의 쿼리는 파라미터 바인딩을 쓰고 있지만, SQL 인젝션, 소스에 박힌 라이브 비밀키, 남의 주문 조회(IDOR), 경로 조작으로 서버 파일 읽기, 음수 수량으로 잔액·재고 조작 등 실서비스에 올리면 바로 악용될 수 있는 결함이 여러 개 있습니다. 결제·재고 로직에도 동시성 문제와 검증 누락이 있어 돈과 재고가 틀어질 수 있습니다.

결론: ❌ 이대로 머지 불가 (Request Changes). 아래 🔴 항목은 이번 주 출시 전에 반드시 고쳐야 합니다.

---

재현 결과 (실제 실행)

시나리오	입력	결과
SQL 인젝션	search_products("' UNION SELECT id,name,password FROM users--")	상품 목록에 사용자 비밀번호(pw_alice, pw_bob)가 섞여 나옴
음수 수량 주문	place_order(bob, pen, -100) (잔액 0원)	재고 5 → 105, bob 잔액 0 → 100,000원
잔액 부족 결제	잔액 0원에서 1,000원 상품 주문	주문 성공, 잔액 -1,000원
IDOR	get_order(1, current_user_id=999)	다른 사용자(user 2)의 주문이 그대로 반환
경로 조작	download_invoice("../shop.db"), download_invoice("/etc/hosts")	DB 파일 전체(20KB) 및 /etc/hosts 내용 반환
페이지네이션	주문 26건인 사용자의 list_orders(uid, page=1)	1~21번째를 건너뛰고 22번부터 5건만 반환
쿠폰	apply_coupon(10000, None) / {"percent":150}	None 반환 / -5000.0 반환
없는 상품 주문	place_order(1, 999, 1)	TypeError: 'NoneType' object is not subscriptable (500 에러)

---

Critical Issues

#	File	Line	Issue	Severity
1	shop.py	19	SQL 인젝션 — keyword를 f-string으로 SQL에 직접 삽입. UNION SELECT로 users 테이블(비밀번호 등) 전체 탈취 가능 (재현됨)	🔴 Critical
2	shop.py	7	라이브 비밀키 하드코딩 — sk_live_... 관리자 토큰이 소스에 노출. 저장소/공유 경로로 유출되면 관리자 권한 탈취. 이미 노출된 키이므로 폐기(rotate) 필요	🔴 Critical
3	shop.py	40–43	IDOR(권한 검증 누락) — current_user_id를 받기만 하고 안 씀. 주문 ID만 바꾸면 누구의 주문이든 조회 가능. SELECT *라 내부 컬럼까지 노출 (재현됨)	🔴 Critical
4	shop.py	77–80	경로 조작(Path Traversal) — ../shop.db로 DB 파일 통째 다운로드, 절대경로(/etc/...)는 os.path.join이 앞부분을 버려서 임의 파일 읽기 가능. 인보이스 소유자 확인도 없음 (재현됨)	🔴 Critical
5	shop.py	46–66	수량 검증 없음 — qty가 음수면 재고는 늘고 잔액은 증가(돈 생성). qty=0이면 빈 주문 생성 (재현됨)	🔴 Critical
6	shop.py	61–64	잔액 확인 없이 차감 — 잔액이 마이너스가 되어도 주문 성공 (재현됨). 존재하지 않는 user_id면 UPDATE가 0행이라 공짜 주문 생성	🔴 Critical
7	shop.py	48–55	재고 경쟁 조건(TOCTOU) — 재고 SELECT 후 별도 UPDATE. Python sqlite3는 DML 직전에야 트랜잭션을 열기 때문에 SELECT는 트랜잭션 밖. 동시 주문 2건이 둘 다 재고 체크를 통과해 초과 판매/재고 음수 발생	🔴 Critical
8	shop.py	25	페이지네이션 off-by-one — offset = page * PAGE_SIZE라 page=1이 첫 20건을 건너뜀. 사용자는 최근(첫 페이지) 주문을 못 봄 (재현됨)	🟠 High
9	shop.py	69–74	쿠폰 예외 삼키기 + 범위 검증 없음 — 오류 시 조용히 None 반환 → 이후 결제 금액 계산이 깨지거나 0원 처리 위험. percent > 100이면 음수 금액, 음수면 가격 인상 (재현됨)	🟠 High
10	shop.py	51	없는 상품 처리 누락 — fetchone()이 None이면 TypeError로 500 에러 (재현됨)	🟠 High
11	shop.py	84	타이밍 공격 가능한 토큰 비교 — == 비교는 일치 길이에 따라 시간이 달라짐. hmac.compare_digest 사용 필요	🟡 Medium
12	shop.py	11–14 (전 함수)	DB 연결 미종료 — 모든 함수가 연결을 열고 닫지 않음. 예외 시 트랜잭션이 열린 채 남아 database is locked 유발 가능	🟡 Medium

---

수정 예시

1. SQL 인젝션 (L19) + 무제한 결과

def search_products(keyword, limit=50):
    escaped = keyword.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
    with closing(get_db()) as db:
        rows = db.execute(
            "SELECT id, name, price FROM products WHERE name LIKE ? ESCAPE '\\' LIMIT ?",
            (f"%{escaped}%", limit),
        ).fetchall()
    return [dict(r) for r in rows]

2. 비밀키 (L7, L84)

import hmac

ADMIN_TOKEN = os.environ["SHOP_ADMIN_TOKEN"]  # 없으면 기동 실패하도록

def is_admin(token):
    return bool(token) and hmac.compare_digest(token, ADMIN_TOKEN)

⚠️ 코드에서 지우는 것만으로는 부족합니다. 이 키는 이미 노출된 것으로 보고 반드시 새로 발급하고 기존 키는 폐기하세요. git에 올라간 적이 있다면 히스토리에도 남아 있습니다.

3. IDOR (L40–43)

def get_order(order_id, current_user_id):
    with closing(get_db()) as db:
        row = db.execute(
            "SELECT id, user_id, created_at FROM orders WHERE id = ? AND user_id = ?",
            (order_id, current_user_id),
        ).fetchone()
    return dict(row) if row else None

4. 경로 조작 (L77–80)

from pathlib import Path
INVOICE_ROOT = Path(INVOICE_DIR).resolve()

def download_invoice(filename, current_user_id):
    path = (INVOICE_ROOT / filename).resolve()
    if not path.is_relative_to(INVOICE_ROOT):   # Python 3.9+
        raise PermissionError("invalid path")
    # + 해당 인보이스가 current_user_id 소유인지 DB로 확인
    return path.read_bytes()

더 안전한 방법은 파일명을 사용자 입력으로 받지 말고 order_id로 받아, 소유권 확인 후 서버가 파일명을 만드는 것입니다.

5~7, 10. 주문 처리 (L46–66) — 검증 + 원자적 처리

def place_order(user_id, product_id, qty):
    if not isinstance(qty, int) or qty <= 0:
        raise ValueError("수량 오류")
    db = get_db()
    db.isolation_level = None
    try:
        db.execute("BEGIN IMMEDIATE")  # 쓰기 잠금을 먼저 잡아 동시 주문 직렬화
        product = db.execute(
            "SELECT price FROM products WHERE id = ?", (product_id,)
        ).fetchone()
        if product is None:
            raise ValueError("상품 없음")
        total = product["price"] * qty

        # 조건부 UPDATE로 체크와 차감을 한 번에
        if db.execute(
            "UPDATE products SET stock = stock - ? WHERE id = ? AND stock >= ?",
            (qty, product_id, qty),
        ).rowcount != 1:
            raise ValueError("재고 부족")
        if db.execute(
            "UPDATE users SET balance = balance - ? WHERE id = ? AND balance >= ?",
            (total, user_id, total),
        ).rowcount != 1:
            raise ValueError("잔액 부족 또는 사용자 없음")

        cur = db.execute("INSERT INTO orders (user_id) VALUES (?)", (user_id,))
        order_id = cur.lastrowid
        db.execute(
            "INSERT INTO order_items (order_id, product_id, qty, price) VALUES (?, ?, ?, ?)",
            (order_id, product_id, qty, product["price"]),
        )
        db.execute("COMMIT")
        return order_id
    except Exception:
        db.execute("ROLLBACK")
        raise
    finally:
        db.close()

추가로 DB 스키마에 CHECK (stock >= 0), CHECK (balance >= 0), CHECK (qty > 0) 제약을 걸어 두면 코드 실수가 있어도 DB가 막아 줍니다. 가격이 소수라면 float 대신 정수(원 단위)로 저장하세요.

8. 페이지네이션 + N+1 (L23–37)

def list_orders(user_id, page=1):
    page = max(1, int(page))
    offset = (page - 1) * PAGE_SIZE
    with closing(get_db()) as db:
        orders = db.execute(
            "SELECT id, created_at FROM orders WHERE user_id = ? "
            "ORDER BY created_at DESC, id DESC LIMIT ? OFFSET ?",
            (user_id, PAGE_SIZE, offset),
        ).fetchall()
        if not orders:
            return []
        ids = [o["id"] for o in orders]
        marks = ",".join("?" * len(ids))
        items = db.execute(
            f"SELECT order_id, product_id, qty, price FROM order_items WHERE order_id IN ({marks})",
            ids,
        ).fetchall()
    by_order = {}
    for i in items:
        by_order.setdefault(i["order_id"], []).append(
            {"product_id": i["product_id"], "qty": i["qty"], "price": i["price"]}
        )
    return [
        {"id": o["id"], "created_at": o["created_at"], "items": by_order.get(o["id"], [])}
        for o in orders
    ]

(marks는 ? 개수만 만드는 것이라 인젝션 위험 없음)

9. 쿠폰 (L69–74)

def apply_coupon(total, coupon):
    if not coupon:
        return total
    percent = coupon.get("percent")
    if not isinstance(percent, (int, float)) or not 0 <= percent <= 100:
        raise ValueError("유효하지 않은 쿠폰")
    return total - total * percent / 100

쿠폰 유효기간·사용 횟수·소유자 검증은 이 함수 밖에서라도 반드시 서버 측에서 해야 합니다.

---

Suggestions

#	File	Line	Suggestion	Category
1	shop.py	32–35	주문마다 아이템 쿼리를 날리는 N+1 쿼리 → IN (...) 한 번으로 묶기 (위 예시)	Performance
2	shop.py	27	ORDER BY 없는 페이지네이션은 순서가 보장되지 않아 페이지 간 중복·누락 발생	Correctness
3	shop.py	19–20	검색 결과 LIMIT 없음 → 상품이 많아지면 응답 폭증. %keyword%는 인덱스를 못 타므로 규모가 커지면 FTS5 검토	Performance
4	shop.py	36	created_at을 조회해 놓고 응답에서 빠뜨림	Correctness
5	shop.py	11–14	get_db()를 context manager로 바꾸고, PRAGMA foreign_keys=ON, busy_timeout 설정	Maintainability
6	shop.py	5–6	DB_PATH, INVOICE_DIR이 상대경로라 실행 위치(cwd)에 따라 다른 DB/폴더를 씀 → 설정/절대경로로	Maintainability
7	shop.py	61–64	결제(잔액 차감) 내역이 기록되지 않음 → 환불·정산·분쟁 대응을 위해 결제/원장(ledger) 테이블 권장	Maintainability
8	전체	—	테스트 없음. 최소한 위 "재현 결과" 표의 시나리오들을 회귀 테스트로 추가 권장	Test coverage
9	전체	—	인증/세션이 이 모듈 밖에서 어떻게 user_id를 넘기는지 확인 필요. 클라이언트가 보낸 user_id를 그대로 쓰면 3번과 같은 문제가 모든 함수에 생김	Security

---

What Looks Good

• list_orders, get_order, place_order는 파라미터 바인딩(?) 을 사용하고 있어 해당 쿼리들은 인젝션에 안전함
• place_order가 여러 쓰기 작업을 마지막에 한 번 commit()하는 구조라, 중간 예외 시 일부만 반영되는 상황은 (연결이 정리되면) 피할 수 있음
• 주문 시점의 가격을 order_items.price에 스냅샷으로 저장 — 이후 상품 가격이 바뀌어도 주문 내역이 보존되는 좋은 설계
• row_factory = sqlite3.Row로 컬럼명 기반 접근 — 가독성 좋음
• 함수 단위로 역할이 잘 나뉘어 있고 짧아서 고치기 쉬운 상태

---

Verdict

❌ Request Changes — 이번 주 출시 불가

출시 전 필수 (🔴): 1. search_products 파라미터 바인딩으로 변경 2. 관리자 토큰을 환경변수로 이동 + 기존 키 폐기·재발급 3. get_order에 소유자 확인 추가 4. download_invoice 경로 검증 + 소유권 확인 5. place_order에 수량·상품·잔액 검증과 원자적 재고/잔액 차감 적용

출시 전 강력 권장 (🟠): 6. 페이지네이션 offset 수정 + ORDER BY 7. apply_coupon 예외 삼키기 제거 및 범위 검증 8. 위 시나리오 회귀 테스트 추가

필수 항목은 대부분 몇 줄짜리 수정이라 하루 안에 끝낼 수 있는 분량입니다. 수정 후 다시 검토받는 것을 권장합니다.