Skip to content

fix(#211): whitelist allowed columns in updateTrack — prevent SQL injection via column names#239

Open
Radexito wants to merge 3 commits into
devfrom
fix/211-sql-injection-whitelist
Open

fix(#211): whitelist allowed columns in updateTrack — prevent SQL injection via column names#239
Radexito wants to merge 3 commits into
devfrom
fix/211-sql-injection-whitelist

Conversation

@Radexito

Copy link
Copy Markdown
Owner

Summary

  • Adds ALLOWED_TRACK_COLUMNS whitelist (35 columns) to trackRepository.js
  • updateTrack() now filters Object.keys(data) through the whitelist before interpolating into the SQL SET clause — unknown keys are logged and dropped
  • Prevents a potential SQL-injection-via-column-name attack through the update-track IPC channel (e.g. from a compromised renderer context)

Root cause

updateTrack() built dynamic SQL using Object.keys(data) directly: ```js
const set = fields.map(f => ${f} = @${f}).join(', ')

If an attacker could call `window.api.updateTrack()` with arbitrary keys, they could inject SQL via identifier names.

All other query paths already use parameterized values (`?` / `@name`) — only column names were vulnerable. The search bar is entirely client-side (no SQL).

## Test plan

- [ ] Run `npm test` — all existing tests pass
- [ ] Manually update a track field (BPM, title, rating) — still works
- [ ] Verify that an unknown key in `updateTrack` produces a console warning and is ignored (no crash)

Closes #211

🤖 Generated with [Claude Code](https://claude.com/claude-code)

…QL injection

The updateTrack() function built SET clauses by interpolating Object.keys()
directly into SQL. An attacker with renderer access could inject arbitrary
column names via the update-track IPC channel. Fix: filter keys through
ALLOWED_TRACK_COLUMNS whitelist before constructing the query; unknown
keys are logged and silently dropped.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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