Skip to content

Commit aeda32c

Browse files
divinity76Niels Dossche
divinity76
and
Niels Dossche
committed
SQLite pdo::quote use ('foo'||x'0000'||'bar') for null bytes
resolves issue phpGH-13952, this is basically a smart_str() version of PR php#13956 per php#13962 (comment) 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 php#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 <nielsdos@php.net>
1 parent 887ed94 commit aeda32c

File tree

2 files changed

+154
-8
lines changed

2 files changed

+154
-8
lines changed

‎ext/pdo_sqlite/sqlite_driver.c

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
#include "php_pdo_sqlite_int.h"
2828
#include "zend_exceptions.h"
2929
#include "sqlite_driver_arginfo.h"
30+
#include "ext/standard/php_string.h"
31+
#include "zend_smart_str.h"
3032

3133
int _pdo_sqlite_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, const char *file, int line) /* {{{ */
3234
{
@@ -219,19 +221,80 @@ static zend_string *pdo_sqlite_last_insert_id(pdo_dbh_t *dbh, const zend_string
219221
return zend_i64_to_str(sqlite3_last_insert_rowid(H->db));
220222
}
221223

222-
/* NB: doesn't handle binary strings... use prepared stmts for that */
223224
static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
224225
{
225-
char *quoted;
226+
const char *ptr = ZSTR_VAL(unquoted);
227+
const char *const end = ZSTR_VAL(unquoted) + ZSTR_LEN(unquoted);
228+
smart_str output = {0};
229+
bool is_in_quote = false;
226230
if (ZSTR_LEN(unquoted) > (INT_MAX - 3) / 2) {
231+
// php_error_docref(NULL, E_WARNING, "String is too long to quote");
227232
return NULL;
228233
}
229-
quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3);
230-
/* TODO use %Q format? */
231-
sqlite3_snprintf(2*ZSTR_LEN(unquoted) + 3, quoted, "'%q'", ZSTR_VAL(unquoted));
232-
zend_string *quoted_str = zend_string_init(quoted, strlen(quoted), 0);
233-
efree(quoted);
234-
return quoted_str;
234+
if(ptr == end) {
235+
smart_str_appendl(&output, "''", 2);
236+
return smart_str_extract(&output);
237+
}
238+
const bool has_null_bytes = memchr(ptr, '\0', ZSTR_LEN(unquoted)) != NULL;
239+
if(has_null_bytes) {
240+
smart_str_appendc(&output, '(');
241+
} else {
242+
// todo: a faster implementation of the loop below is possible when we know there are no null bytes
243+
// (but lets just keep it simple while fixing the problem, and worry about micro-optimizations later)
244+
}
245+
while (ptr < end) {
246+
// \x00 and ' needs special handling
247+
const char special_characters[2] = "'\0";
248+
const size_t bytes_until_special = php_strcspn(ptr, special_characters, end, special_characters + sizeof(special_characters));
249+
if (bytes_until_special) {
250+
if(!is_in_quote) {
251+
if(ptr != ZSTR_VAL(unquoted)) {
252+
smart_str_appendl(&output, "||", 2);
253+
}
254+
smart_str_appendc(&output, '\'');
255+
is_in_quote = true;
256+
}
257+
smart_str_appendl(&output, ptr, bytes_until_special);
258+
ptr += bytes_until_special;
259+
ZEND_ASSERT(ptr <= end);
260+
if(ptr == end) {
261+
break;
262+
}
263+
}
264+
if(*ptr == '\'') {
265+
if(!is_in_quote) {
266+
if(ptr != ZSTR_VAL(unquoted)) {
267+
smart_str_appendl(&output, "||", 2);
268+
}
269+
smart_str_appendc(&output, '\'');
270+
is_in_quote = true;
271+
}
272+
const size_t number_of_consecutive_single_quotes = php_strspn(ptr, special_characters, end, special_characters + 1);
273+
smart_str_appendl(&output, ptr, number_of_consecutive_single_quotes);
274+
smart_str_appendl(&output, ptr, number_of_consecutive_single_quotes);
275+
ptr += number_of_consecutive_single_quotes;
276+
} else {
277+
ZEND_ASSERT(*ptr == '\0');
278+
if(is_in_quote) {
279+
smart_str_appendl(&output, "'||", 3);
280+
is_in_quote = false;
281+
}
282+
const size_t number_of_consecutive_nulls = php_strspn(ptr, special_characters + 1, end, special_characters + 2); // ...
283+
smart_str_appendl(&output, "x'", 2);
284+
for(size_t i = 0; i < number_of_consecutive_nulls; ++i) {
285+
smart_str_appendl(&output, "00", 2);
286+
}
287+
smart_str_appendc(&output, '\'');
288+
ptr += number_of_consecutive_nulls;
289+
}
290+
}
291+
if(is_in_quote) {
292+
smart_str_appendc(&output, '\'');
293+
}
294+
if(has_null_bytes) {
295+
smart_str_appendc(&output, ')');
296+
}
297+
return smart_str_extract(&output);
235298
}
236299

237300
static bool sqlite_handle_begin(pdo_dbh_t *dbh)

‎ext/pdo_sqlite/tests/gh13952.phpt

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
--TEST--
2+
GH-13952 (sqlite PDO::quote silently corrupts strings with null bytes)
3+
--EXTENSIONS--
4+
pdo
5+
pdo_sqlite
6+
--FILE--
7+
<?php
8+
$db = new \PDO('sqlite::memory:', null, null, array(
9+
\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
10+
\PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_ASSOC,
11+
\PDO::ATTR_EMULATE_PREPARES => false,
12+
));
13+
14+
$test_cases = [
15+
"",
16+
"x",
17+
"\x00",
18+
"a\x00b",
19+
"\x00\x00\x00",
20+
"foobar",
21+
"foo'''bar",
22+
"'foo'''bar'",
23+
"'foo'\x00'bar'",
24+
"foo\x00\x00\x00bar",
25+
"\x00foo\x00\x00\x00bar\x00",
26+
"\x00\x00\x00foo",
27+
"foo\x00\x00\x00",
28+
"\x80", // << invalid UTF8
29+
"\x00\x80\x00", // << invalid UTF8
30+
];
31+
32+
foreach($test_cases as $test){
33+
$res = $db->query("SELECT " . $db->quote($test))->fetch($db::FETCH_NUM)[0] === $test;
34+
if(!$res){
35+
throw new Exception("Failed for $test");
36+
}
37+
}
38+
39+
$db->exec('CREATE TABLE test (name TEXT)');
40+
41+
foreach ($test_cases as $test_case) {
42+
$quoted = $db->quote($test_case);
43+
echo trim(json_encode($test_case, JSON_PARTIAL_OUTPUT_ON_ERROR), '"'), " -> $quoted\n";
44+
$db->exec("INSERT INTO test (name) VALUES (" . $quoted . ")");
45+
}
46+
47+
$stmt = $db->prepare('SELECT * from test');
48+
$stmt->execute();
49+
foreach ($stmt->fetchAll() as $result) {
50+
var_dump($result['name']);
51+
}
52+
?>
53+
--EXPECTF--
54+
-> ''
55+
x -> 'x'
56+
\u0000 -> (x'00')
57+
a\u0000b -> ('a'||x'00'||'b')
58+
\u0000\u0000\u0000 -> (x'000000')
59+
foobar -> 'foobar'
60+
foo'''bar -> 'foo''''''bar'
61+
'foo'''bar' -> '''foo''''''bar'''
62+
'foo'\u0000'bar' -> ('''foo'''||x'00'||'''bar''')
63+
foo\u0000\u0000\u0000bar -> ('foo'||x'000000'||'bar')
64+
\u0000foo\u0000\u0000\u0000bar\u0000 -> (x'00'||'foo'||x'000000'||'bar'||x'00')
65+
\u0000\u0000\u0000foo -> (x'000000'||'foo')
66+
foo\u0000\u0000\u0000 -> ('foo'||x'000000')
67+
null -> '€'
68+
null -> (x'00'||'€'||x'00')
69+
string(0) ""
70+
string(1) "x"
71+
string(1) "%0"
72+
string(3) "a%0b"
73+
string(3) "%0%0%0"
74+
string(6) "foobar"
75+
string(9) "foo'''bar"
76+
string(11) "'foo'''bar'"
77+
string(11) "'foo'%0'bar'"
78+
string(9) "foo%0%0%0bar"
79+
string(11) "%0foo%0%0%0bar%0"
80+
string(6) "%0%0%0foo"
81+
string(6) "foo%0%0%0"
82+
string(1) "€"
83+
string(3) "%0€%0"

0 commit comments

Comments
 (0)