Skip to content

fix SQL injection - #29

Merged
kuaukutsu merged 4 commits into
mainfrom
feature/fix-sql-injection
Mar 26, 2026
Merged

kuaukutsu merged 4 commits into
mainfrom
feature/fix-sql-injection

Conversation

@kuaukutsu

@kuaukutsu kuaukutsu commented Mar 26, 2026

Copy link
Copy Markdown
Owner

fix: parameterized queries in migration tracking table operations

Replace sprintf-interpolated SQL with named placeholders (:name, :version,
:atime) in INSERT/DELETE statements within Command::up() and Command::down().
Table name remains injected via sprintf but is now validated in Config
constructor against /^\w+$/.

Refactoring:

  • Extract shared fetchRecord/exec logic into abstract Statement class;
    Transaction and TransactionMysql now extend it
  • Add ThrowPrepareException trait with prepareException(PDO $connection): never
    to eliminate duplicated error-handling code between Connection and Statement
  • Add ErrorInfo value object for consistent PDO::errorInfo() formatting
  • Add PrepareException for prepare() failures
  • Add FactoryTransaction interface; Type::makeFactoryTransaction() replaces
    the if/else branch in Connection::beginTransaction()

Bug fixes:

  • generateVersion(): replace substr(..., 0, -1) with explicit substr(..., 0, 12)
  • filesystem/Action::down(): use joinFilename() instead of string concatenation

Tests:

  • StatementTest: parameterized exec/fetchRecord, SQL injection via bound params
  • ConfigurationTest: valid and invalid table name cases

@kuaukutsu kuaukutsu self-assigned this Mar 26, 2026
@kuaukutsu
kuaukutsu merged commit 7198be6 into main Mar 26, 2026
5 checks passed
@kuaukutsu
kuaukutsu deleted the feature/fix-sql-injection branch March 26, 2026 19:18
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