Skip to content

Commit b5dfeb5

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 5efe334 commit b5dfeb5

2 files changed

Lines changed: 113 additions & 14 deletions

File tree

‎tests/WP_SQLite_Translator_Tests.php‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3593,4 +3593,113 @@ function ( $row ) {
35933593
// Verify autoincrement detection works with backtick-quoted identifiers.
35943594
$this->assertStringContainsString( 'AUTO_INCREMENT', $create_sql );
35953595
}
3596+
3597+
public function testDoubleQuotedStringsAreParameterized() {
3598+
$this->assertQuery( 'INSERT INTO _options (option_name, option_value) VALUES ("dq_name", "dq_value")' );
3599+
3600+
// The double-quoted strings should be bound as parameters, not inlined.
3601+
$insert_query = null;
3602+
foreach ( $this->engine->executed_sqlite_queries as $q ) {
3603+
if ( stripos( $q['sql'], 'INSERT' ) !== false && stripos( $q['sql'], '_options' ) !== false ) {
3604+
$insert_query = $q;
3605+
}
3606+
}
3607+
$this->assertNotNull( $insert_query );
3608+
$this->assertNotEmpty( $insert_query['params'], 'Double-quoted strings should be bound as parameters' );
3609+
$this->assertStringNotContainsString( 'dq_name', $insert_query['sql'], 'Value should not appear in SQL' );
3610+
$this->assertStringNotContainsString( 'dq_value', $insert_query['sql'], 'Value should not appear in SQL' );
3611+
$this->assertContains( 'dq_name', $insert_query['params'] );
3612+
$this->assertContains( 'dq_value', $insert_query['params'] );
3613+
3614+
// Verify the data was inserted correctly.
3615+
$result = $this->assertQuery( 'SELECT * FROM _options WHERE option_name = "dq_name"' );
3616+
$this->assertCount( 1, $result );
3617+
$this->assertEquals( 'dq_value', $result[0]->option_value );
3618+
}
3619+
3620+
public function testDoubleQuotedStringWithBackslashEscapeDoesNotCauseInjection() {
3621+
// In MySQL, \" inside double-quoted strings is an escaped double quote.
3622+
// The MySQL lexer produces a single token: "admin\" OR 1=1--"
3623+
// with value: admin" OR 1=1--
3624+
//
3625+
// Without parameterization, passing the raw token to SQLite would be:
3626+
// "admin\" OR 1=1--" (SQLite sees "admin\" as identifier + SQL)
3627+
//
3628+
// With parameterization, the value is safely bound as a parameter.
3629+
$this->assertQuery(
3630+
'INSERT INTO _options (option_name, option_value) VALUES ("safe_key", "admin\" OR 1=1--")'
3631+
);
3632+
3633+
// Verify the injection payload is not present in the SQL sent to SQLite.
3634+
$insert_query = null;
3635+
foreach ( $this->engine->executed_sqlite_queries as $q ) {
3636+
if ( stripos( $q['sql'], 'INSERT' ) !== false && stripos( $q['sql'], '_options' ) !== false ) {
3637+
$insert_query = $q;
3638+
}
3639+
}
3640+
$this->assertNotNull( $insert_query );
3641+
$this->assertStringNotContainsString( 'OR 1=1', $insert_query['sql'], 'Injection payload should not appear in SQL' );
3642+
$this->assertNotEmpty( $insert_query['params'], 'Values should be bound as parameters' );
3643+
3644+
$result = $this->assertQuery( 'SELECT * FROM _options WHERE option_name = "safe_key"' );
3645+
$this->assertCount( 1, $result );
3646+
$this->assertEquals( 'admin" OR 1=1--', $result[0]->option_value );
3647+
}
3648+
3649+
public function testDateFormatWithSingleQuotesInFormat() {
3650+
$this->assertQuery(
3651+
'CREATE TABLE _tmp_dates (
3652+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3653+
created_at DATETIME NOT NULL
3654+
);'
3655+
);
3656+
$this->assertQuery( "INSERT INTO _tmp_dates (created_at) VALUES ('2024-01-15 10:30:00')" );
3657+
3658+
// DATE_FORMAT with a format that produces a value — verify it works.
3659+
$result = $this->assertQuery(
3660+
"SELECT DATE_FORMAT(created_at, '%Y-%m-%d') as formatted FROM _tmp_dates"
3661+
);
3662+
$this->assertCount( 1, $result );
3663+
$this->assertEquals( '2024-01-15', $result[0]->formatted );
3664+
}
3665+
3666+
public function testIntervalExpression() {
3667+
$this->assertQuery(
3668+
'CREATE TABLE _tmp_dates (
3669+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3670+
created_at DATETIME NOT NULL
3671+
);'
3672+
);
3673+
$this->assertQuery( 'INSERT INTO _tmp_dates (created_at) VALUES (\'2024-01-15 10:30:00\')' );
3674+
3675+
$result = $this->assertQuery(
3676+
'SELECT DATE_ADD(created_at, INTERVAL 1 DAY) as future_date FROM _tmp_dates'
3677+
);
3678+
$this->assertCount( 1, $result );
3679+
$this->assertEquals( '2024-01-16 10:30:00', $result[0]->future_date );
3680+
3681+
$result = $this->assertQuery(
3682+
'SELECT DATE_SUB(created_at, INTERVAL 1 DAY) as past_date FROM _tmp_dates'
3683+
);
3684+
$this->assertCount( 1, $result );
3685+
$this->assertEquals( '2024-01-14 10:30:00', $result[0]->past_date );
3686+
}
3687+
3688+
public function testLikeBinaryWithSingleQuoteInPattern() {
3689+
$this->assertQuery(
3690+
"CREATE TABLE _tmp_table (
3691+
ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
3692+
name varchar(50) NOT NULL default ''
3693+
);"
3694+
);
3695+
3696+
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('it''s a test')" );
3697+
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('no quote here')" );
3698+
3699+
$result = $this->assertQuery(
3700+
"SELECT * FROM _tmp_table WHERE name LIKE BINARY 'it''s%'"
3701+
);
3702+
$this->assertCount( 1, $result );
3703+
$this->assertEquals( "it's a test", $result[0]->name );
3704+
}
35963705
}

‎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)