Skip to content

Commit 4cf8fb3

Browse files
iluuu1994iluuu1994
committed
Merge branch 'PHP-8.3' into PHP-8.4
* PHP-8.3: GHSA-w476-322c-wpvm: [pdo_firebird] Fix SQL injection via NUL bytes in quoted strings
2 parents a2ee688 + 0842621 commit 4cf8fb3

2 files changed

Lines changed: 90 additions & 25 deletions

File tree

‎ext/pdo_firebird/firebird_driver.c‎

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ static FbTokenType php_firebird_get_token(const char** begin, const char* end)
295295
return ret;
296296
}
297297

298-
static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTable* named_params)
298+
static int php_firebird_preprocess(const zend_string* sql, char* sql_out, size_t* sql_out_len, HashTable* named_params)
299299
{
300300
bool passAsIs = 1, execBlock = 0;
301301
zend_long pindex = -1;
@@ -326,7 +326,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
326326
if (l > 252) {
327327
return 0;
328328
}
329-
strncpy(ident, i, l);
329+
memcpy(ident, i, l);
330330
ident[l] = '\0';
331331
if (!strcasecmp(ident, "EXECUTE"))
332332
{
@@ -351,7 +351,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
351351
if (l > 252) {
352352
return 0;
353353
}
354-
strncpy(ident2, i2, l);
354+
memcpy(ident2, i2, l);
355355
ident2[l] = '\0';
356356
execBlock = !strcasecmp(ident2, "BLOCK");
357357
passAsIs = 0;
@@ -367,22 +367,28 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
367367

368368
if (passAsIs)
369369
{
370-
strcpy(sql_out, ZSTR_VAL(sql));
370+
memcpy(sql_out, ZSTR_VAL(sql), ZSTR_LEN(sql));
371+
sql_out[ZSTR_LEN(sql)] = '\0';
372+
*sql_out_len = ZSTR_LEN(sql);
371373
return 1;
372374
}
373375

374-
strncat(sql_out, start, p - start);
376+
char *sql_out_p = sql_out;
377+
memcpy(sql_out_p, start, p - start);
378+
sql_out_p += p - start;
375379

376380
while (p < end)
377381
{
378382
start = p;
379383
tok = php_firebird_get_token(&p, end);
380384
switch (tok)
381385
{
382-
case ttParamMark:
383-
tok = php_firebird_get_token(&p, end);
386+
case ttParamMark: {
387+
const char* p_peek = p;
388+
tok = php_firebird_get_token(&p_peek, end);
384389
if (tok == ttIdent /*|| tok == ttString*/)
385390
{
391+
p = p_peek;
386392
++pindex;
387393
l = p - start;
388394
/* check the length of the identifier */
@@ -391,7 +397,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
391397
if (l > 253) {
392398
return 0;
393399
}
394-
strncpy(pname, start, l);
400+
memcpy(pname, start, l);
395401
pname[l] = '\0';
396402

397403
if (named_params) {
@@ -400,7 +406,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
400406
zend_hash_str_update(named_params, pname, l, &tmp);
401407
}
402408

403-
strcat(sql_out, "?");
409+
*sql_out_p++ = '?';
404410
}
405411
else
406412
{
@@ -410,10 +416,11 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
410416
return 0;
411417
}
412418
++pindex;
413-
strncat(sql_out, start, p - start);
419+
memcpy(sql_out_p, start, p - start);
420+
sql_out_p += p - start;
414421
}
415422
break;
416-
423+
}
417424
case ttIdent:
418425
if (execBlock)
419426
{
@@ -425,11 +432,14 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
425432
if (l > 252) {
426433
return 0;
427434
}
428-
strncpy(ident, start, l);
435+
memcpy(ident, start, l);
429436
ident[l] = '\0';
430437
if (!strcasecmp(ident, "AS"))
431438
{
432-
strncat(sql_out, start, end - start);
439+
memcpy(sql_out_p, start, end - start);
440+
sql_out_p += end - start;
441+
*sql_out_p = '\0';
442+
*sql_out_len = sql_out_p - sql_out;
433443
return 1;
434444
}
435445
}
@@ -440,7 +450,8 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
440450
case ttComment:
441451
case ttString:
442452
case ttOther:
443-
strncat(sql_out, start, p - start);
453+
memcpy(sql_out_p, start, p - start);
454+
sql_out_p += p - start;
444455
break;
445456

446457
case ttBrokenComment:
@@ -458,6 +469,8 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
458469
break;
459470
}
460471
}
472+
*sql_out_p = '\0';
473+
*sql_out_len = sql_out_p - sql_out;
461474
return 1;
462475
}
463476

@@ -792,7 +805,7 @@ static zend_long firebird_handle_doer(pdo_dbh_t *dbh, const zend_string *sql) /*
792805
static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
793806
{
794807
size_t qcount = 0;
795-
char const *co, *l, *r;
808+
char const *co, *l;
796809
char *c;
797810
size_t quotedlen;
798811
zend_string *quoted_str;
@@ -801,9 +814,15 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un
801814
return ZSTR_INIT_LITERAL("''", 0);
802815
}
803816

817+
const char * const end = ZSTR_VAL(unquoted) + ZSTR_LEN(unquoted);
818+
804819
/* Firebird only requires single quotes to be doubled if string lengths are used */
805820
/* count the number of ' characters */
806-
for (co = ZSTR_VAL(unquoted); (co = strchr(co,'\'')); qcount++, co++);
821+
for (co = ZSTR_VAL(unquoted); co < end; co++) {
822+
if (*co == '\'') {
823+
qcount++;
824+
}
825+
}
807826

808827
if (UNEXPECTED(ZSTR_LEN(unquoted) + 2 > ZSTR_MAX_LEN - qcount)) {
809828
return NULL;
@@ -815,15 +834,14 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un
815834
*c++ = '\'';
816835

817836
/* foreach (chunk that ends in a quote) */
818-
for (l = ZSTR_VAL(unquoted); (r = strchr(l,'\'')); l = r+1) {
819-
strncpy(c, l, r-l+1);
820-
c += (r-l+1);
821-
/* add the second quote */
822-
*c++ = '\'';
837+
for (l = ZSTR_VAL(unquoted); l < end; l++) {
838+
*c++ = *l;
839+
if (*l == '\'') {
840+
/* add the second quote */
841+
*c++ = '\'';
842+
}
823843
}
824844

825-
/* copy the remainder */
826-
strncpy(c, l, quotedlen-(c-ZSTR_VAL(quoted_str))-1);
827845
ZSTR_VAL(quoted_str)[quotedlen-1] = '\'';
828846
ZSTR_VAL(quoted_str)[quotedlen] = '\0';
829847

@@ -1001,6 +1019,7 @@ static int php_firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const zend_string *sq
10011019
{
10021020
pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
10031021
char *new_sql;
1022+
size_t new_sql_len;
10041023

10051024
/* allocate the statement */
10061025
if (isc_dsql_allocate_statement(H->isc_status, &H->db, s)) {
@@ -1012,14 +1031,14 @@ static int php_firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const zend_string *sq
10121031
we need to replace :foo by ?, and store the name we just replaced */
10131032
new_sql = emalloc(ZSTR_LEN(sql)+1);
10141033
new_sql[0] = '\0';
1015-
if (!php_firebird_preprocess(sql, new_sql, named_params)) {
1034+
if (!php_firebird_preprocess(sql, new_sql, &new_sql_len, named_params)) {
10161035
php_firebird_error_with_info(dbh, "07000", strlen("07000"), NULL, 0);
10171036
efree(new_sql);
10181037
return 0;
10191038
}
10201039

10211040
/* prepare the statement */
1022-
if (isc_dsql_prepare(H->isc_status, &H->tr, s, 0, new_sql, H->sql_dialect, out_sqlda)) {
1041+
if (isc_dsql_prepare(H->isc_status, &H->tr, s, new_sql_len, new_sql, H->sql_dialect, out_sqlda)) {
10231042
php_firebird_error(dbh);
10241043
efree(new_sql);
10251044
return 0;

‎ext/pdo_firebird/tests/ghsa-w476-322c-wpvm.phpt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
GHSA-w476-322c-wpvm: SQL injection in pdo_firebird via NUL bytes in quoted strings
3+
--EXTENSIONS--
4+
pdo_firebird
5+
--SKIPIF--
6+
<?php require('skipif.inc'); ?>
7+
--XLEAK--
8+
A bug in firebird causes a memory leak when calling `isc_attach_database()`.
9+
See https://github.com/FirebirdSQL/firebird/issues/7849
10+
--FILE--
11+
<?php
12+
13+
require("testdb.inc");
14+
15+
$dbh = getDbConnection();
16+
$dbh->exec('CREATE TABLE ghsa_w476_322c_wpvm (name VARCHAR(255))');
17+
18+
$param = $dbh->quote("\0");
19+
$param2 = $dbh->quote('or 1=1--');
20+
var_export($param);
21+
echo("\n");
22+
23+
echo "prepare: ";
24+
$stmt = $dbh->prepare("SELECT * FROM ghsa_w476_322c_wpvm WHERE name = {$param} AND name = {$param2}");
25+
$stmt->execute();
26+
echo json_encode($stmt->fetchAll(PDO::FETCH_ASSOC)) . "\n";
27+
28+
echo "query: ";
29+
$stmt = $dbh->query("SELECT * FROM ghsa_w476_322c_wpvm WHERE name = {$param} AND name = {$param2}");
30+
echo json_encode($stmt->fetchAll(PDO::FETCH_ASSOC)) . "\n";
31+
32+
echo "exec: ";
33+
$affectedRows = $dbh->exec("UPDATE ghsa_w476_322c_wpvm SET name = 'updated' WHERE name = {$param} AND name = {$param2}");
34+
echo $affectedRows . "\n";
35+
?>
36+
--CLEAN--
37+
<?php
38+
require 'testdb.inc';
39+
$dbh = getDbConnection();
40+
$dbh->exec("DROP TABLE ghsa_w476_322c_wpvm");
41+
?>
42+
--EXPECT--
43+
'\'' . "\0" . '\''
44+
prepare: []
45+
query: []
46+
exec: 0

0 commit comments

Comments
 (0)