Skip to content

fix(routes): resolve DoS via SQLite SQLITE_LIMIT_VARIABLE_NUMBER overflow in DELETE /tasks/bulk (#313)#320

Open
arpanmukherjee38 wants to merge 1 commit into
utksh1:mainfrom
arpanmukherjee38:fix/sqlite-bulk-delete-limit
Open

fix(routes): resolve DoS via SQLite SQLITE_LIMIT_VARIABLE_NUMBER overflow in DELETE /tasks/bulk (#313)#320
arpanmukherjee38 wants to merge 1 commit into
utksh1:mainfrom
arpanmukherjee38:fix/sqlite-bulk-delete-limit

Conversation

@arpanmukherjee38
Copy link
Copy Markdown

Summary

Fixes #313

DELETE /tasks/bulk accepted an unbounded list of task IDs and passed
them all directly into a parameterized WHERE id IN (?, ?, ...) query.
SQLite enforces a hard limit of SQLITE_LIMIT_VARIABLE_NUMBER = 999.
Sending 1000+ IDs triggered an unhandled OperationalError that
propagated as a 500 response, leaking internal database details and
providing a reliable denial-of-service vector requiring zero
authentication.


Root Cause

Two separate code paths were vulnerable:

  1. bulk_delete_tasks() — no length validation on the incoming
    task_ids list before building the placeholder string.
  2. delete_task_records() — the shared helper also built an unbounded
    placeholder string, meaning any caller (including clear_all_tasks)
    inherited the same crash vector.

Changes

backend/secuscan/models.py

  • Added MAX_BULK_DELETE = 500 constant (safely under the SQLite limit)
  • Added BulkDeleteRequest Pydantic model with Field(min_length=1, max_length=500) — FastAPI rejects oversized requests with a
    structured 422 Unprocessable Entity before the function body runs

backend/secuscan/routes.py

  • Updated bulk_delete_tasks() to accept BulkDeleteRequest as the
    request body; Pydantic enforces the size cap at parse time
  • Refactored delete_task_records() to process IDs in chunks of
    SQLITE_CHUNK_SIZE = 500, staying safely under the SQLite variable
    limit for all callers including clear_all_tasks
  • Fixed duplicate from .models import block; consolidated into a
    single import statement

Before / After

Before

@router.delete("/tasks/bulk")
async def bulk_delete_tasks(task_ids: List[str]):
    # no length check — crashes at 1000+ IDs
    placeholders = ",".join(["?"] * len(task_ids))
    await db.execute(f"DELETE FROM tasks WHERE id IN ({placeholders})", task_ids)

After

@router.delete("/tasks/bulk")
async def bulk_delete_tasks(request: BulkDeleteRequest):
    # Pydantic enforces max 500 IDs before this runs
    task_ids = request.task_ids
    ...
    await delete_task_records(task_ids)  # chunked internally

Testing

import requests, uuid

# Should now return 422, not 500
ids = [str(uuid.uuid4()) for _ in range(1200)]
r = requests.delete(
    "http://localhost:8000/api/v1/tasks/bulk",
    json={"task_ids": ids}
)
assert r.status_code == 422

# Should work fine within the limit
ids = [str(uuid.uuid4()) for _ in range(500)]
r = requests.delete(
    "http://localhost:8000/api/v1/tasks/bulk",
    json={"task_ids": ids}
)
assert r.status_code == 200

Security Impact Closed

  • ✅ DoS via unbounded SQL variable list — eliminated
  • ✅ Raw OperationalError leak in response body — eliminated
  • ✅ Shared helper delete_task_records() hardened for all callers

@arpanmukherjee38 arpanmukherjee38 force-pushed the fix/sqlite-bulk-delete-limit branch 3 times, most recently from d251f21 to 929c9dc Compare May 26, 2026 10:27
@utksh1 utksh1 added area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests type:security Security work category bonus label type:bug Bug fix work category bonus label level:critical 80 pts difficulty label for critical or high-impact PRs labels May 26, 2026
Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes. The chunking/limit approach is the right direction for the SQLite variable-limit DoS, but this needs regression tests before merge: empty list, 500 IDs accepted, 501 IDs rejected, and delete_task_records chunking over more than one chunk. Please also keep formatting consistent on the long DELETE statements.

@arpanmukherjee38 arpanmukherjee38 force-pushed the fix/sqlite-bulk-delete-limit branch from 929c9dc to f01418a Compare May 27, 2026 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests level:critical 80 pts difficulty label for critical or high-impact PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][Advanced] DELETE /tasks/bulk has no list-size cap, crashes with SQLite SQLITE_LIMIT_VARIABLE_NUMBER at 1000+ IDs

2 participants