Skip to content

Fix the two CodeQL log-injection alerts - #34

Open
dav3exe wants to merge 3 commits into
startnextgen:mainfrom
Cybertron-HNG:main
Open

dav3exe wants to merge 3 commits into
startnextgen:mainfrom
Cybertron-HNG:main

Conversation

@dav3exe

@dav3exe dav3exe commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Fixes the two open CodeQL alerts in this repo's Security tab (py/log-injection, app/routers/games.py, lines 118 and 517).

The game name from a request was written to the log as it came. It now goes through _plain, which drops carriage returns and newlines, so a request can't fake another log entry. Game names are already limited to letters, numbers and hyphens, so nothing was exploitable.

One file changed, plus a test. It went through a PR into the fork's dev (#142) and dev into main (#143) with pytest, malware-scan, CodeQL and the dependency checks green; 0 open alerts on the PR.

Checks run locally: pytest 1004 passed, 2 skipped.

If pytest shows a failure that ran for about 15 minutes, it's a hung GitHub runner (the job is cancelled, not failed): re-run it. The same job passed on the fork after a re-run.

…log lines

The game name and the cover's game name are logged when an upload fails; they now go through _plain, which drops carriage returns and newlines, so a request can't fake another log entry.
Fix the two CodeQL log-injection alerts
Merge dev into main: CodeQL log-injection fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant