Skip to content

Commit 2f325db

Browse files
JanJakesJanJakes
committed
Improve string escaping in the legacy SQLite translator
Use parameterized queries and PDO::quote() consistently for all string literals processed by the translator.
1 parent 10be91a commit 2f325db

2 files changed

Lines changed: 102 additions & 14 deletions

File tree

‎tests/WP_SQLite_Translator_Tests.php‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3557,4 +3557,102 @@ public function testCreateTableWithDefaultNowFunction() {
35573557
$result = $this->assertQuery( 'SELECT * FROM test_now_default WHERE id = 2' );
35583558
$this->assertRegExp( '/\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d/', $result[0]->updated );
35593559
}
3560+
3561+
public function testDoubleQuotedStringsAreParameterized() {
3562+
$this->assertQuery( 'INSERT INTO _options (option_name, option_value) VALUES ("dq_name", "dq_value")' );
3563+
3564+
// The double-quoted strings should be bound as parameters, not inlined.
3565+
$insert_query = null;
3566+
foreach ( $this->engine->executed_sqlite_queries as $q ) {
3567+
if ( stripos( $q['sql'], 'INSERT' ) !== false && stripos( $q['sql'], '_options' ) !== false ) {
3568+
$insert_query = $q;
3569+
}
3570+
}
3571+
$this->assertNotNull( $insert_query );
3572+
$this->assertNotEmpty( $insert_query['params'], 'Double-quoted strings should be bound as parameters' );
3573+
$this->assertStringNotContainsString( 'dq_name', $insert_query['sql'], 'Value should not appear in SQL' );
3574+
$this->assertStringNotContainsString( 'dq_value', $insert_query['sql'], 'Value should not appear in SQL' );
3575+
$this->assertContains( 'dq_name', $insert_query['params'] );
3576+
$this->assertContains( 'dq_value', $insert_query['params'] );
3577+
3578+
// Verify the data was inserted correctly.
3579+
$result = $this->assertQuery( 'SELECT * FROM _options WHERE option_name = "dq_name"' );
3580+
$this->assertCount( 1, $result );
3581+
$this->assertEquals( 'dq_value', $result[0]->option_value );
3582+
}
3583+
3584+
public function testDoubleQuotedStringWithBackslashEscapeDoesNotCauseInjection() {
3585+
// In MySQL, \" inside double-quoted strings is an escaped double quote.
3586+
// The MySQL lexer produces a single token: "admin\" OR 1=1--"
3587+
// with value: admin" OR 1=1--
3588+
//
3589+
// Without parameterization, passing the raw token to SQLite would be:
3590+
// "admin\" OR 1=1--" (SQLite sees "admin\" as identifier + SQL)
3591+
//
3592+
// With parameterization, the value is safely bound as a parameter.
3593+
$this->assertQuery(
3594+
'INSERT INTO _options (option_name, option_value) VALUES ("safe_key", "admin\" OR 1=1--")'
3595+
);
3596+
3597+
$result = $this->assertQuery( 'SELECT * FROM _options WHERE option_name = "safe_key"' );
3598+
$this->assertCount( 1, $result );
3599+
$this->assertEquals( 'admin" OR 1=1--', $result[0]->option_value );
3600+
}
3601+
3602+
public function testDateFormatWithSingleQuotesInFormat() {
3603+
$this->assertQuery(
3604+
'CREATE TABLE _tmp_dates (
3605+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3606+
created_at DATETIME NOT NULL
3607+
);'
3608+
);
3609+
$this->assertQuery( "INSERT INTO _tmp_dates (created_at) VALUES ('2024-01-15 10:30:00')" );
3610+
3611+
// DATE_FORMAT with a format that produces a value — verify it works.
3612+
$result = $this->assertQuery(
3613+
"SELECT DATE_FORMAT(created_at, '%Y-%m-%d') as formatted FROM _tmp_dates"
3614+
);
3615+
$this->assertCount( 1, $result );
3616+
$this->assertEquals( '2024-01-15', $result[0]->formatted );
3617+
}
3618+
3619+
public function testIntervalExpression() {
3620+
$this->assertQuery(
3621+
'CREATE TABLE _tmp_dates (
3622+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3623+
created_at DATETIME NOT NULL
3624+
);'
3625+
);
3626+
$this->assertQuery( 'INSERT INTO _tmp_dates (created_at) VALUES (\'2024-01-15 10:30:00\')' );
3627+
3628+
$result = $this->assertQuery(
3629+
'SELECT DATE_ADD(created_at, INTERVAL 1 DAY) as future_date FROM _tmp_dates'
3630+
);
3631+
$this->assertCount( 1, $result );
3632+
$this->assertEquals( '2024-01-16 10:30:00', $result[0]->future_date );
3633+
3634+
$result = $this->assertQuery(
3635+
'SELECT DATE_SUB(created_at, INTERVAL 1 DAY) as past_date FROM _tmp_dates'
3636+
);
3637+
$this->assertCount( 1, $result );
3638+
$this->assertEquals( '2024-01-14 10:30:00', $result[0]->past_date );
3639+
}
3640+
3641+
public function testLikeBinaryWithSingleQuoteInPattern() {
3642+
$this->assertQuery(
3643+
"CREATE TABLE _tmp_table (
3644+
ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
3645+
name varchar(50) NOT NULL default ''
3646+
);"
3647+
);
3648+
3649+
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('it''s a test')" );
3650+
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('no quote here')" );
3651+
3652+
$result = $this->assertQuery(
3653+
"SELECT * FROM _tmp_table WHERE name LIKE BINARY 'it''s%'"
3654+
);
3655+
$this->assertCount( 1, $result );
3656+
$this->assertEquals( "it's a test", $result[0]->name );
3657+
}
35603658
}

‎wp-includes/sqlite/class-wp-sqlite-translator.php‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2251,10 +2251,7 @@ private function remember_last_reserved_keyword( $token ) {
22512251
* @return bool True if the parameter was extracted successfully, false otherwise.
22522252
*/
22532253
private function extract_bound_parameter( $token, &$params ) {
2254-
if ( ! $token->matches(
2255-
WP_SQLite_Token::TYPE_STRING,
2256-
WP_SQLite_Token::FLAG_STRING_SINGLE_QUOTES
2257-
)
2254+
if ( ! $token->matches( WP_SQLite_Token::TYPE_STRING )
22582255
|| 'AS' === $this->last_reserved_keyword
22592256
) {
22602257
return false;
@@ -2539,7 +2536,7 @@ private function translate_date_format( $token ) {
25392536

25402537
$this->rewriter->add( new WP_SQLite_Token( 'STRFTIME', WP_SQLite_Token::TYPE_KEYWORD, WP_SQLite_Token::FLAG_KEYWORD_FUNCTION ) );
25412538
$this->rewriter->add( new WP_SQLite_Token( '(', WP_SQLite_Token::TYPE_OPERATOR ) );
2542-
$this->rewriter->add( new WP_SQLite_Token( "'$new_format'", WP_SQLite_Token::TYPE_STRING ) );
2539+
$this->rewriter->add( new WP_SQLite_Token( $this->pdo->quote( $new_format ), WP_SQLite_Token::TYPE_STRING ) );
25432540
$this->rewriter->add( new WP_SQLite_Token( ',', WP_SQLite_Token::TYPE_OPERATOR ) );
25442541

25452542
// Add the buffered tokens back to the stream.
@@ -2614,7 +2611,7 @@ private function translate_interval( $token ) {
26142611
}
26152612
}
26162613

2617-
$this->rewriter->add( new WP_SQLite_Token( "'{$interval_op}$num $unit'", WP_SQLite_Token::TYPE_STRING ) );
2614+
$this->rewriter->add( new WP_SQLite_Token( $this->pdo->quote( "{$interval_op}$num $unit" ), WP_SQLite_Token::TYPE_STRING ) );
26182615
return true;
26192616
}
26202617

@@ -2712,16 +2709,9 @@ private function translate_like_binary( $token ): bool {
27122709
* @return string The escaped GLOB pattern.
27132710
*/
27142711
private function escape_like_to_glob( $pattern ) {
2715-
// Remove surrounding quotes
2716-
$pattern = trim( $pattern, "'\"" );
2717-
27182712
$pattern = str_replace( '%', '*', $pattern );
27192713
$pattern = str_replace( '_', '?', $pattern );
2720-
2721-
// No need to escape special characters in this case
2722-
// because GLOB doesn't require escaping in the same way LIKE does
2723-
// Return the pattern wrapped in single quotes
2724-
return "'" . $pattern . "'";
2714+
return $this->pdo->quote( $pattern );
27252715
}
27262716

27272717
/**

0 commit comments

Comments
 (0)