Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent SQL injection in overwrite.c #354

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

devin-ai-integration[bot]
Copy link

@devin-ai-integration devin-ai-integration bot commented Dec 20, 2024

Fix SQL injection vulnerability identified in security scanning alert #29 by implementing SQLite parser-based validation.

Changes:

  • Replace string-based SQL validation with SQLite's built-in parser
  • Use sqlite3_prepare_v2() to parse and validate SQL syntax
  • Verify single-statement queries using pzTail parameter
  • Ensure only read-only (SELECT) statements are allowed via sqlite3_stmt_readonly()
  • Support flexible column naming (col/column and val/value)
  • Validate presence of required columns with naming variants
  • Make timestamp column optional to maintain compatibility
  • Add proper error handling and statement cleanup
  • Maintain existing error flow and default query fallback

The changes are minimal and focused on security, avoiding any build system modifications. The implementation balances security with existing functionality by supporting multiple column naming conventions and making the timestamp column optional.

Testing:

  • Validates proper SELECT statements with required columns
  • Supports both "col"/"column" and "val"/"value" column names
  • Maintains compatibility with --no-timestamp flag
  • Rejects multi-statement SQL injection attempts
  • Prevents non-SELECT statements
  • Maintains backward compatibility with existing queries
  • Proper cleanup of SQLite resources

Link to Devin run: https://app.devin.ai/sessions/7bc67faddc4d4c96ad62208b48c20218

- Add SQL query validation function get_safe_sql_query
- Validate all SQL queries before execution
- Block dangerous SQL tokens and enforce SELECT queries
- Ensure queries only access overwrites table
- Provide safe default query fallback

Fixes security scanning alert #29

Co-Authored-By: Matt Wong <matt@liquidaty.com>
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Add "(aside)" to your comment to have me ignore it.

devin-ai-integration bot and others added 11 commits December 20, 2024 19:09
- Fix line wrapping for default query string
- Adjust line continuation style for strstr conditions
- Correct function parameter alignment
- Follow project's clang-format style

Co-Authored-By: Matt Wong <matt@liquidaty.com>
…ation

Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.com>
- Replace string-based SQL validation with SQLite parser
- Add proper error handling for invalid queries
- Maintain existing error flow and ok flag logic
- Keep changes minimal and focused on security fix

Link to Devin run: https://app.devin.ai/sessions/7bc67faddc4d4c96ad62208b48c20218

Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.com>
…ed columns

Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.com>
Co-Authored-By: Matt Wong <matt@liquidaty.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.

0 participants