From aeda32cef6787f35e84984027b0ee9f86c7cb6c1 Mon Sep 17 00:00:00 2001 From: hanshenrik Date: Mon, 15 Apr 2024 17:25:36 +0200 Subject: [PATCH] SQLite pdo::quote use ('foo'||x'0000'||'bar') for null bytes resolves issue GH-13952, this is basically a smart_str() version of PR #13956 per https://github.com/php/php-src/pull/13962#issuecomment-2056696180 this is 1 of the 4 proposed alternatives to the problem, and the pros of this solution is that it produces smaller queries than the alternatives, and retains the sqlite datatype 'string' (instead of changing it to blob), and should make PDO::quote faster as we now avoid the overhead of copying data to/from sqlite3_snprintf. The cons of this solution, that I can think of right now, is that the implementation is non-trivial, involves a bunch of php-allocator-reallocs() (PR #13956 does not invovle reallocs, as it pre-computes the length. also worth noting that php allocator's reallocs() are faster than libc realloc, often avoiding talking to the OS), and SQLite's LENGTH(('foo'||x'00'||'bar')) returns 3 instead of 7, and binary strings gets the datatype 'string' instead of 'blob' (that can be considered both a pro and a con) Co-authored-by: Niels Dossche --- ext/pdo_sqlite/sqlite_driver.c | 79 ++++++++++++++++++++++++++--- ext/pdo_sqlite/tests/gh13952.phpt | 83 +++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 ext/pdo_sqlite/tests/gh13952.phpt diff --git a/ext/pdo_sqlite/sqlite_driver.c b/ext/pdo_sqlite/sqlite_driver.c index 7cd8880b86099..4d5048d09caa8 100644 --- a/ext/pdo_sqlite/sqlite_driver.c +++ b/ext/pdo_sqlite/sqlite_driver.c @@ -27,6 +27,8 @@ #include "php_pdo_sqlite_int.h" #include "zend_exceptions.h" #include "sqlite_driver_arginfo.h" +#include "ext/standard/php_string.h" +#include "zend_smart_str.h" int _pdo_sqlite_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, const char *file, int line) /* {{{ */ { @@ -219,19 +221,80 @@ static zend_string *pdo_sqlite_last_insert_id(pdo_dbh_t *dbh, const zend_string return zend_i64_to_str(sqlite3_last_insert_rowid(H->db)); } -/* NB: doesn't handle binary strings... use prepared stmts for that */ static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype) { - char *quoted; + const char *ptr = ZSTR_VAL(unquoted); + const char *const end = ZSTR_VAL(unquoted) + ZSTR_LEN(unquoted); + smart_str output = {0}; + bool is_in_quote = false; if (ZSTR_LEN(unquoted) > (INT_MAX - 3) / 2) { + // php_error_docref(NULL, E_WARNING, "String is too long to quote"); return NULL; } - quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3); - /* TODO use %Q format? */ - sqlite3_snprintf(2*ZSTR_LEN(unquoted) + 3, quoted, "'%q'", ZSTR_VAL(unquoted)); - zend_string *quoted_str = zend_string_init(quoted, strlen(quoted), 0); - efree(quoted); - return quoted_str; + if(ptr == end) { + smart_str_appendl(&output, "''", 2); + return smart_str_extract(&output); + } + const bool has_null_bytes = memchr(ptr, '\0', ZSTR_LEN(unquoted)) != NULL; + if(has_null_bytes) { + smart_str_appendc(&output, '('); + } else { + // todo: a faster implementation of the loop below is possible when we know there are no null bytes + // (but lets just keep it simple while fixing the problem, and worry about micro-optimizations later) + } + while (ptr < end) { + // \x00 and ' needs special handling + const char special_characters[2] = "'\0"; + const size_t bytes_until_special = php_strcspn(ptr, special_characters, end, special_characters + sizeof(special_characters)); + if (bytes_until_special) { + if(!is_in_quote) { + if(ptr != ZSTR_VAL(unquoted)) { + smart_str_appendl(&output, "||", 2); + } + smart_str_appendc(&output, '\''); + is_in_quote = true; + } + smart_str_appendl(&output, ptr, bytes_until_special); + ptr += bytes_until_special; + ZEND_ASSERT(ptr <= end); + if(ptr == end) { + break; + } + } + if(*ptr == '\'') { + if(!is_in_quote) { + if(ptr != ZSTR_VAL(unquoted)) { + smart_str_appendl(&output, "||", 2); + } + smart_str_appendc(&output, '\''); + is_in_quote = true; + } + const size_t number_of_consecutive_single_quotes = php_strspn(ptr, special_characters, end, special_characters + 1); + smart_str_appendl(&output, ptr, number_of_consecutive_single_quotes); + smart_str_appendl(&output, ptr, number_of_consecutive_single_quotes); + ptr += number_of_consecutive_single_quotes; + } else { + ZEND_ASSERT(*ptr == '\0'); + if(is_in_quote) { + smart_str_appendl(&output, "'||", 3); + is_in_quote = false; + } + const size_t number_of_consecutive_nulls = php_strspn(ptr, special_characters + 1, end, special_characters + 2); // ... + smart_str_appendl(&output, "x'", 2); + for(size_t i = 0; i < number_of_consecutive_nulls; ++i) { + smart_str_appendl(&output, "00", 2); + } + smart_str_appendc(&output, '\''); + ptr += number_of_consecutive_nulls; + } + } + if(is_in_quote) { + smart_str_appendc(&output, '\''); + } + if(has_null_bytes) { + smart_str_appendc(&output, ')'); + } + return smart_str_extract(&output); } static bool sqlite_handle_begin(pdo_dbh_t *dbh) diff --git a/ext/pdo_sqlite/tests/gh13952.phpt b/ext/pdo_sqlite/tests/gh13952.phpt new file mode 100644 index 0000000000000..65692b3a3ff19 --- /dev/null +++ b/ext/pdo_sqlite/tests/gh13952.phpt @@ -0,0 +1,83 @@ +--TEST-- +GH-13952 (sqlite PDO::quote silently corrupts strings with null bytes) +--EXTENSIONS-- +pdo +pdo_sqlite +--FILE-- + \PDO::ERRMODE_EXCEPTION, + \PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_ASSOC, + \PDO::ATTR_EMULATE_PREPARES => false, +)); + +$test_cases = [ + "", + "x", + "\x00", + "a\x00b", + "\x00\x00\x00", + "foobar", + "foo'''bar", + "'foo'''bar'", + "'foo'\x00'bar'", + "foo\x00\x00\x00bar", + "\x00foo\x00\x00\x00bar\x00", + "\x00\x00\x00foo", + "foo\x00\x00\x00", + "\x80", // << invalid UTF8 + "\x00\x80\x00", // << invalid UTF8 +]; + +foreach($test_cases as $test){ + $res = $db->query("SELECT " . $db->quote($test))->fetch($db::FETCH_NUM)[0] === $test; + if(!$res){ + throw new Exception("Failed for $test"); + } +} + +$db->exec('CREATE TABLE test (name TEXT)'); + +foreach ($test_cases as $test_case) { + $quoted = $db->quote($test_case); + echo trim(json_encode($test_case, JSON_PARTIAL_OUTPUT_ON_ERROR), '"'), " -> $quoted\n"; + $db->exec("INSERT INTO test (name) VALUES (" . $quoted . ")"); +} + +$stmt = $db->prepare('SELECT * from test'); +$stmt->execute(); +foreach ($stmt->fetchAll() as $result) { + var_dump($result['name']); +} +?> +--EXPECTF-- +-> '' +x -> 'x' +\u0000 -> (x'00') +a\u0000b -> ('a'||x'00'||'b') +\u0000\u0000\u0000 -> (x'000000') +foobar -> 'foobar' +foo'''bar -> 'foo''''''bar' +'foo'''bar' -> '''foo''''''bar''' +'foo'\u0000'bar' -> ('''foo'''||x'00'||'''bar''') +foo\u0000\u0000\u0000bar -> ('foo'||x'000000'||'bar') +\u0000foo\u0000\u0000\u0000bar\u0000 -> (x'00'||'foo'||x'000000'||'bar'||x'00') +\u0000\u0000\u0000foo -> (x'000000'||'foo') +foo\u0000\u0000\u0000 -> ('foo'||x'000000') +null -> '€' +null -> (x'00'||'€'||x'00') +string(0) "" +string(1) "x" +string(1) "%0" +string(3) "a%0b" +string(3) "%0%0%0" +string(6) "foobar" +string(9) "foo'''bar" +string(11) "'foo'''bar'" +string(11) "'foo'%0'bar'" +string(9) "foo%0%0%0bar" +string(11) "%0foo%0%0%0bar%0" +string(6) "%0%0%0foo" +string(6) "foo%0%0%0" +string(1) "€" +string(3) "%0€%0"