Skip to content

Commit ab048bd

Browse files
iluuu1994iluuu1994
committed
Fix SQL injection in ext-pgsql via E'...' backslash breakout
php_pgsql_add_quotes() quotes the string with E'...', but PQescapeStringConn() does not escape \ unless standard_conforming_strings is off. PQescapeStringConn() is meant to be used with '', so do that instead. Fixes GHSA-7qpv-r5mr-78m4
1 parent fcd691b commit ab048bd

8 files changed

Lines changed: 81 additions & 10 deletions

File tree

‎ext/pgsql/pgsql.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4469,7 +4469,7 @@ static int php_pgsql_convert_match(const zend_string *str, const char *regex , s
44694469
*/
44704470
static zend_string *php_pgsql_add_quotes(zend_string *src)
44714471
{
4472-
return zend_string_concat3("E'", strlen("E'"), ZSTR_VAL(src), ZSTR_LEN(src), "'", strlen("'"));
4472+
return zend_string_concat3("'", strlen("'"), ZSTR_VAL(src), ZSTR_LEN(src), "'", strlen("'"));
44734473
}
44744474
/* }}} */
44754475

@@ -4747,7 +4747,6 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
47474747
zend_string *str;
47484748
/* PostgreSQL ignores \0 */
47494749
str = zend_string_alloc(Z_STRLEN_P(val) * 2, 0);
4750-
/* better to use PGSQLescapeLiteral since PGescapeStringConn does not handle special \ */
47514750
ZSTR_LEN(str) = PQescapeStringConn(pg_link, ZSTR_VAL(str),
47524751
Z_STRVAL_P(val), Z_STRLEN_P(val), &escape_err);
47534752
if (escape_err) {

‎ext/pgsql/tests/10pg_convert_9.phpt‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ $converted = pg_convert($db, $table_name, $fields);
2121

2222
var_dump($converted);
2323

24+
var_dump(pg_convert($db, $table_name, ['str' => "\\' OR 1=1"]));
25+
2426
/* Invalid values */
2527
try {
2628
$converted = pg_convert($db, $table_name, [5 => 'AAA']);
@@ -48,18 +50,30 @@ try {
4850
} catch (\TypeError $e) {
4951
echo $e->getMessage(), \PHP_EOL;
5052
}
53+
54+
/* standard_conforming_strings = 1 */
55+
pg_query($db, "SET standard_conforming_strings = 1");
56+
var_dump(pg_convert($db, $table_name, ['str' => "\\' OR 1=1"]));
5157
?>
5258
--EXPECT--
5359
array(3) {
5460
[""num""]=>
5561
string(4) "1234"
5662
[""str""]=>
57-
string(6) "E'AAA'"
63+
string(5) "'AAA'"
5864
[""bin""]=>
59-
string(12) "E'\\x424242'"
65+
string(11) "'\\x424242'"
66+
}
67+
array(1) {
68+
[""str""]=>
69+
string(13) "'\\'' OR 1=1'"
6070
}
6171
Array of values must be an associative array with string keys
6272
Array of values must be an associative array with string keys
6373
Values must be of type string|int|float|bool|null, array given
6474
Values must be of type string|int|float|bool|null, stdClass given
6575
Values must be of type string|int|float|bool|null, PgSql\Connection given
76+
array(1) {
77+
[""str""]=>
78+
string(12) "'\'' OR 1=1'"
79+
}

‎ext/pgsql/tests/10pg_convert_json_array.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ if (!pg_insert($db, $table_name_92, $fields)) {
3232
--EXPECT--
3333
array(2) {
3434
[""textary""]=>
35-
string(51) "E'{"meeting", "lunch", "training", "presentation"}'"
35+
string(50) "'{"meeting", "lunch", "training", "presentation"}'"
3636
[""jsn""]=>
37-
string(22) "E'{"f1":1,"f2":"foo"}'"
37+
string(21) "'{"f1":1,"f2":"foo"}'"
3838
}
3939
OK

‎ext/pgsql/tests/12pg_insert_9.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ try {
5454
echo "Ok\n";
5555
?>
5656
--EXPECTF--
57-
INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES (1234,E'AAA',E'\\x424242');
57+
INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES (1234,'AAA','\\x424242');
5858
INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES ('1234','AAA','BBB');
5959
object(PgSql\Result)#%d (0) {
6060
}

‎ext/pgsql/tests/14pg_update_9.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ echo pg_update($db, $table_name, $fields, $ids, PGSQL_DML_STRING|PGSQL_DML_ESCAP
2626
echo "Ok\n";
2727
?>
2828
--EXPECT--
29-
UPDATE "php_pgsql_test" SET "num"=1234,"str"=E'ABC',"bin"=E'\\x58595a' WHERE "num"=1234;
29+
UPDATE "php_pgsql_test" SET "num"=1234,"str"='ABC',"bin"='\\x58595a' WHERE "num"=1234;
3030
UPDATE "php_pgsql_test" SET "num"='1234',"str"='ABC',"bin"='XYZ' WHERE "num"='1234';
3131
Ok

‎ext/pgsql/tests/GHSA-7qpv-r5mr-78m4.phpt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
--TEST--
2+
GHSA-7qpv-r5mr-78m4: SQL injection via E'...' backslash breakout
3+
--CREDITS--
4+
expatch.llc
5+
--EXTENSIONS--
6+
pgsql
7+
--SKIPIF--
8+
<?php include("skipif.inc"); ?>
9+
--FILE--
10+
<?php
11+
include 'config.inc';
12+
13+
$db = pg_connect($conn_str);
14+
15+
pg_query($db, "SET standard_conforming_strings = 1");
16+
17+
pg_query($db, "DROP TABLE IF EXISTS ghsa_7qpv_r5mr_78m4");
18+
pg_query($db, "CREATE TABLE ghsa_7qpv_r5mr_78m4 (id serial primary key, name text, admin boolean)");
19+
pg_query($db, "INSERT INTO ghsa_7qpv_r5mr_78m4 (name, admin) VALUES ('alice', false), ('bob', false)");
20+
21+
$params = ['name' => "zzz' OR 1=1 --"];
22+
echo pg_select($db, 'ghsa_7qpv_r5mr_78m4', $params, PGSQL_DML_STRING) . "\n";
23+
printf("returned: %d\n\n", count(pg_select($db, 'ghsa_7qpv_r5mr_78m4', $params)));
24+
25+
$params = ['name' => "zzz\\' OR 1=1 --"];
26+
echo pg_select($db, 'ghsa_7qpv_r5mr_78m4', $params, PGSQL_DML_STRING) . "\n";
27+
printf("returned: %d\n\n", count(pg_select($db, 'ghsa_7qpv_r5mr_78m4', $params)));
28+
29+
$params = ['name' => "john\\', true) --", 'admin' => 'false'];
30+
echo pg_insert($db, 'ghsa_7qpv_r5mr_78m4', $params, PGSQL_DML_STRING) . "\n";
31+
pg_insert($db, 'ghsa_7qpv_r5mr_78m4', $params);
32+
var_dump(pg_select($db, 'ghsa_7qpv_r5mr_78m4', ['id' => 3])[0]['admin']);
33+
echo "\n";
34+
35+
$params = ['name' => "jake\\', true) --", 'admin' => 'f'];
36+
echo pg_insert($db, 'ghsa_7qpv_r5mr_78m4', $params, PGSQL_DML_ESCAPE|PGSQL_DML_STRING) . "\n";
37+
pg_insert($db, 'ghsa_7qpv_r5mr_78m4', $params, PGSQL_DML_EXEC|PGSQL_DML_ESCAPE);
38+
var_dump(pg_select($db, 'ghsa_7qpv_r5mr_78m4', ['id' => 4])[0]['admin']);
39+
40+
?>
41+
--EXPECT--
42+
SELECT * FROM "ghsa_7qpv_r5mr_78m4" WHERE "name"='zzz'' OR 1=1 --';
43+
returned: 0
44+
45+
SELECT * FROM "ghsa_7qpv_r5mr_78m4" WHERE "name"='zzz\'' OR 1=1 --';
46+
returned: 0
47+
48+
INSERT INTO "ghsa_7qpv_r5mr_78m4" ("name","admin") VALUES ('john\'', true) --','f');
49+
string(1) "f"
50+
51+
INSERT INTO "ghsa_7qpv_r5mr_78m4" ("name","admin") VALUES ('jake\'', true) --','f');
52+
string(1) "f"
53+
--CLEAN--
54+
<?php
55+
include('config.inc');
56+
$db = pg_connect($conn_str);
57+
pg_query($db, "DROP TABLE IF EXISTS ghsa_7qpv_r5mr_78m4");
58+
?>

‎ext/pgsql/tests/bug64609.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,5 @@ var_dump($converted);
2828
--EXPECT--
2929
array(1) {
3030
[""a""]=>
31-
string(5) "E'ok'"
31+
string(4) "'ok'"
3232
}

‎ext/pgsql/tests/bug68638.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ pg_query($conn, "DROP TABLE $table");
3434

3535
?>
3636
--EXPECT--
37-
string(52) "UPDATE "test_68638" SET "value"=E'inf' WHERE "id"=1;"
37+
string(51) "UPDATE "test_68638" SET "value"='inf' WHERE "id"=1;"
3838
array(2) {
3939
["id"]=>
4040
string(1) "1"

0 commit comments

Comments
 (0)