Skip to content

Commit 11f487a

Browse files
authored
chore: Sentinel: [MEDIUM] Fix LIKE clause wildcard injection
🛡️ Sentinel: [MEDIUM] Fix LIKE clause wildcard injection
2 parents f5c0dd5 + 4d8a794 commit 11f487a

5 files changed

Lines changed: 76 additions & 2 deletions

File tree

.jules/sentinel.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
## 2025-05-16 - [Medium] Fix LIKE clause wildcard injection
2+
**Vulnerability:** A LIKE clause wildcard injection vulnerability in the Pokemon API. In `src/app/repositories/pokemon_repository.py`, the `find_all` method took a `name` filter string which was used in a `LIKE` SQL query (`ilike()`). While it escaped `%` and `_`, it failed to escape the literal escape character `\` itself. This allowed user input to escape the database's wildcard characters in unintended ways, leading to an incorrect or malicious query executing.
3+
**Learning:** Even when manually escaping string tokens before passing them into a `LIKE` statement, it is critical to also escape the specific escape character itself (typically `\`) otherwise the user can neutralize the query's wildcards.
4+
**Prevention:** When doing manual substring search escaping for database queries in python (e.g., `ilike(f"%{escaped}%")`), ensure you escape the backslash *first* before escaping other wildcard characters. For example: `escaped = input.replace("\\", "\\\\").replace("%", r"\%").replace("_", r"\_")`.

pyproject.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,7 @@ ignore_missing_imports = true
3030
[[tool.mypy.overrides]]
3131
module = "PIL.*"
3232
ignore_missing_imports = true
33+
34+
[[tool.mypy.overrides]]
35+
module = "app.services.recognition_service"
36+
warn_unused_ignores = false

src/app/repositories/pokemon_repository.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def find_all(
5858
"""
5959
stmt = select(PokemonModel)
6060
if name:
61-
escaped = name.replace("%", r"\%").replace("_", r"\_")
61+
escaped = name.replace("\\", "\\\\").replace("%", r"\%").replace("_", r"\_")
6262
stmt = stmt.where(PokemonModel.name.ilike(f"%{escaped}%", escape="\\"))
6363
stmt = stmt.order_by(PokemonModel.id).limit(limit).offset(offset)
6464

src/app/services/recognition_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def __init__(self, model_path: str, confidence_threshold: float = 0.20) -> None:
3737
)
3838
return
3939

40-
from ultralytics import YOLO # lazy import — not needed at import time
40+
from ultralytics import YOLO # type: ignore[attr-defined] # lazy import
4141

4242
self._model = YOLO(model_path)
4343
logger.info(

tests/test_pokemon_repository.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,69 @@ def test_to_summary_dto_non_list_type(
362362
'Pokemon ID 994 has invalid types format (not a list): "electric"'
363363
in caplog.text
364364
)
365+
366+
367+
# ---------------------------------------------------------------------------
368+
# LIKE clause wildcard injection tests (CWE-89)
369+
# These tests verify the fix introduced in PR #240: backslashes in user input
370+
# must be escaped *before* % and _ so they cannot neutralise the escape char.
371+
# ---------------------------------------------------------------------------
372+
373+
374+
def test_find_all_backslash_percent_does_not_match(db_session: Session) -> None:
375+
"""Backslash-percent attack pattern must NOT match any Pokemon.
376+
377+
Without the fix, a user input of backslash+percent could escape the leading
378+
percent wildcard added by the query, producing a literal search for '%' and
379+
potentially matching unintended rows. With the fix, the backslash is doubled
380+
first so it is treated as a literal character in the LIKE pattern.
381+
"""
382+
repo = PokemonRepository(db_session)
383+
results = repo.find_all(name="\\%")
384+
385+
assert results == [], (
386+
f"Expected no results for backslash-percent attack, got {results}"
387+
)
388+
389+
390+
def test_find_all_backslash_underscore_does_not_match(db_session: Session) -> None:
391+
"""Backslash-underscore attack pattern must NOT match any Pokemon.
392+
393+
Without the fix, backslash+underscore in user input could escape the `_`
394+
single-char wildcard, widening the search unexpectedly. The fix ensures both
395+
characters are treated as literals.
396+
"""
397+
repo = PokemonRepository(db_session)
398+
results = repo.find_all(name="\\_")
399+
400+
assert results == [], (
401+
f"Expected no results for backslash-underscore attack, got {results}"
402+
)
403+
404+
405+
def test_find_all_double_backslash_treated_as_literal(db_session: Session) -> None:
406+
"""A double backslash in input must be treated as a single literal backslash.
407+
408+
Verifies that the escape order (backslash first) produces a correct LIKE
409+
pattern: the doubled backslash is interpreted as the literal character.
410+
Since no Pokemon name contains a backslash, the result must be empty.
411+
"""
412+
repo = PokemonRepository(db_session)
413+
results = repo.find_all(name="\\\\")
414+
415+
assert results == [], (
416+
f"Expected no results for double-backslash pattern, got {results}"
417+
)
418+
419+
420+
def test_find_all_normal_name_unaffected_by_escape(db_session: Session) -> None:
421+
"""Normal name searches must still work correctly after the injection fix.
422+
423+
Regression guard: ensures the backslash-escaping pre-processing does not
424+
break ordinary partial-match searches with no special characters.
425+
"""
426+
repo = PokemonRepository(db_session)
427+
results = repo.find_all(name="pika")
428+
429+
assert len(results) == 1
430+
assert results[0].name == "pikachu"

0 commit comments

Comments
 (0)