From a03f7d5cb1980a378f5790fc51f1edb16b7f4e4b Mon Sep 17 00:00:00 2001 From: hanshenrik Date: Mon, 15 Apr 2024 13:18:30 +0200 Subject: [PATCH 1/2] Use x'hex' syntax for null bytes in pdo::quote Implements x'hex' encoding in pdo::quote for handling strings with null bytes, providing a reliable workaround for issue GH-13952. An alternative fix is discussed in PR #13956 PR #13956 does something interesting, it avoids the overhead of copying to/from sqlite3_snprintf, probably speeding up PDO::quote, but right now I just want to keep things simple. Co-authored-by: Niels Dossche --- ext/pdo_sqlite/sqlite_driver.c | 18 +++++++ ext/pdo_sqlite/tests/gh13952.phpt | 83 +++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) 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..70594620db0a9 100644 --- a/ext/pdo_sqlite/sqlite_driver.c +++ b/ext/pdo_sqlite/sqlite_driver.c @@ -226,6 +226,24 @@ static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unqu if (ZSTR_LEN(unquoted) > (INT_MAX - 3) / 2) { return NULL; } + if(ZSTR_LEN(unquoted) != 0 && memchr(ZSTR_VAL(unquoted), '\0', ZSTR_LEN(unquoted))) { + // (''||x'hex') + // the odd (''||) thing is to make sure quote produce a sqlite datatype "string" rather than "blob" ... + // https://github.com/php/php-src/pull/13962/files#r1565485792 + zend_string *quoted = zend_string_safe_alloc(9 + (2 * ZSTR_LEN(unquoted)), 1, 0, 0); + char *outptr = ZSTR_VAL(quoted); + const char *inptr = ZSTR_VAL(unquoted); + const char *const inendptr = inptr + ZSTR_LEN(unquoted); + memcpy(outptr, "(''||x'", 7); + outptr += 7; + while(inptr != inendptr) { + const unsigned char c = *inptr++; + *outptr++ = "0123456789ABCDEF"[c >> 4]; + *outptr++ = "0123456789ABCDEF"[c & 0x0F]; + } + memcpy(outptr, "')", 3); // todo: does zend_string_safe_alloc write the null terminator? if it does, reduce this to 2 + return quoted; + } quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3); /* TODO use %Q format? */ sqlite3_snprintf(2*ZSTR_LEN(unquoted) + 3, quoted, "'%q'", ZSTR_VAL(unquoted)); diff --git a/ext/pdo_sqlite/tests/gh13952.phpt b/ext/pdo_sqlite/tests/gh13952.phpt new file mode 100644 index 0000000000000..467579b92bcc1 --- /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 -> (''||x'610062') +\u0000\u0000\u0000 -> (''||x'000000') +foobar -> 'foobar' +foo'''bar -> 'foo''''''bar' +'foo'''bar' -> '''foo''''''bar''' +'foo'\u0000'bar' -> (''||x'27666F6F27002762617227') +foo\u0000\u0000\u0000bar -> (''||x'666F6F000000626172') +\u0000foo\u0000\u0000\u0000bar\u0000 -> (''||x'00666F6F00000062617200') +\u0000\u0000\u0000foo -> (''||x'000000666F6F') +foo\u0000\u0000\u0000 -> (''||x'666F6F000000') +null -> '€' +null -> (''||x'008000') +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" From aa1cdeff0330f677095ecac7593dd75f14103008 Mon Sep 17 00:00:00 2001 From: hanshenrik Date: Mon, 15 Apr 2024 18:02:31 +0200 Subject: [PATCH 2/2] remove datatype text workaround ref https://github.com/php/php-src/pull/13956#issuecomment-2057160576 --- ext/pdo_sqlite/sqlite_driver.c | 13 ++++++------- ext/pdo_sqlite/tests/gh13952.phpt | 18 +++++++++--------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/ext/pdo_sqlite/sqlite_driver.c b/ext/pdo_sqlite/sqlite_driver.c index 70594620db0a9..6ed050c8570fc 100644 --- a/ext/pdo_sqlite/sqlite_driver.c +++ b/ext/pdo_sqlite/sqlite_driver.c @@ -227,21 +227,20 @@ static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unqu return NULL; } if(ZSTR_LEN(unquoted) != 0 && memchr(ZSTR_VAL(unquoted), '\0', ZSTR_LEN(unquoted))) { - // (''||x'hex') - // the odd (''||) thing is to make sure quote produce a sqlite datatype "string" rather than "blob" ... - // https://github.com/php/php-src/pull/13962/files#r1565485792 - zend_string *quoted = zend_string_safe_alloc(9 + (2 * ZSTR_LEN(unquoted)), 1, 0, 0); + // x'hex' + zend_string *quoted = zend_string_safe_alloc(3 + (2 * ZSTR_LEN(unquoted)), 1, 0, 0); char *outptr = ZSTR_VAL(quoted); const char *inptr = ZSTR_VAL(unquoted); const char *const inendptr = inptr + ZSTR_LEN(unquoted); - memcpy(outptr, "(''||x'", 7); - outptr += 7; + *outptr++ = 'x'; + *outptr++ = '\''; while(inptr != inendptr) { const unsigned char c = *inptr++; *outptr++ = "0123456789ABCDEF"[c >> 4]; *outptr++ = "0123456789ABCDEF"[c & 0x0F]; } - memcpy(outptr, "')", 3); // todo: does zend_string_safe_alloc write the null terminator? if it does, reduce this to 2 + *outptr++ = '\''; + *outptr = '\0'; // does zend_string_safe_alloc write the null terminator? if it does, remove this line return quoted; } quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3); diff --git a/ext/pdo_sqlite/tests/gh13952.phpt b/ext/pdo_sqlite/tests/gh13952.phpt index 467579b92bcc1..1f7440b335c1f 100644 --- a/ext/pdo_sqlite/tests/gh13952.phpt +++ b/ext/pdo_sqlite/tests/gh13952.phpt @@ -53,19 +53,19 @@ foreach ($stmt->fetchAll() as $result) { --EXPECTF-- -> '' x -> 'x' -\u0000 -> (''||x'00') -a\u0000b -> (''||x'610062') -\u0000\u0000\u0000 -> (''||x'000000') +\u0000 -> x'00' +a\u0000b -> x'610062' +\u0000\u0000\u0000 -> x'000000' foobar -> 'foobar' foo'''bar -> 'foo''''''bar' 'foo'''bar' -> '''foo''''''bar''' -'foo'\u0000'bar' -> (''||x'27666F6F27002762617227') -foo\u0000\u0000\u0000bar -> (''||x'666F6F000000626172') -\u0000foo\u0000\u0000\u0000bar\u0000 -> (''||x'00666F6F00000062617200') -\u0000\u0000\u0000foo -> (''||x'000000666F6F') -foo\u0000\u0000\u0000 -> (''||x'666F6F000000') +'foo'\u0000'bar' -> x'27666F6F27002762617227' +foo\u0000\u0000\u0000bar -> x'666F6F000000626172' +\u0000foo\u0000\u0000\u0000bar\u0000 -> x'00666F6F00000062617200' +\u0000\u0000\u0000foo -> x'000000666F6F' +foo\u0000\u0000\u0000 -> x'666F6F000000' null -> '€' -null -> (''||x'008000') +null -> x'008000' string(0) "" string(1) "x" string(1) "%0"