Skip to content

Commit 3e57595

Browse files
iluuu1994iluuu1994
committed
Merge branch 'PHP-8.5'
* PHP-8.5: GHSA-w476-322c-wpvm: [pdo_firebird] Fix SQL injection via NUL bytes in quoted strings
2 parents 257cb3a + a02c0ce commit 3e57595

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
@@ -293,7 +293,7 @@ static FbTokenType php_firebird_get_token(const char** begin, const char* end)
293293
return ret;
294294
}
295295

296-
static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTable* named_params)
296+
static int php_firebird_preprocess(const zend_string* sql, char* sql_out, size_t* sql_out_len, HashTable* named_params)
297297
{
298298
bool passAsIs = true, execBlock = false;
299299
zend_long pindex = -1;
@@ -324,7 +324,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
324324
if (l > 252) {
325325
return 0;
326326
}
327-
strncpy(ident, i, l);
327+
memcpy(ident, i, l);
328328
ident[l] = '\0';
329329
if (!strcasecmp(ident, "EXECUTE"))
330330
{
@@ -349,7 +349,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
349349
if (l > 252) {
350350
return 0;
351351
}
352-
strncpy(ident2, i2, l);
352+
memcpy(ident2, i2, l);
353353
ident2[l] = '\0';
354354
execBlock = !strcasecmp(ident2, "BLOCK");
355355
passAsIs = false;
@@ -365,22 +365,28 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
365365

366366
if (passAsIs)
367367
{
368-
strcpy(sql_out, ZSTR_VAL(sql));
368+
memcpy(sql_out, ZSTR_VAL(sql), ZSTR_LEN(sql));
369+
sql_out[ZSTR_LEN(sql)] = '\0';
370+
*sql_out_len = ZSTR_LEN(sql);
369371
return 1;
370372
}
371373

372-
strncat(sql_out, start, p - start);
374+
char *sql_out_p = sql_out;
375+
memcpy(sql_out_p, start, p - start);
376+
sql_out_p += p - start;
373377

374378
while (p < end)
375379
{
376380
start = p;
377381
tok = php_firebird_get_token(&p, end);
378382
switch (tok)
379383
{
380-
case ttParamMark:
381-
tok = php_firebird_get_token(&p, end);
384+
case ttParamMark: {
385+
const char* p_peek = p;
386+
tok = php_firebird_get_token(&p_peek, end);
382387
if (tok == ttIdent /*|| tok == ttString*/)
383388
{
389+
p = p_peek;
384390
++pindex;
385391
l = p - start;
386392
/* check the length of the identifier */
@@ -389,7 +395,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
389395
if (l > 253) {
390396
return 0;
391397
}
392-
strncpy(pname, start, l);
398+
memcpy(pname, start, l);
393399
pname[l] = '\0';
394400

395401
if (named_params) {
@@ -398,7 +404,7 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
398404
zend_hash_str_update(named_params, pname, l, &tmp);
399405
}
400406

401-
strcat(sql_out, "?");
407+
*sql_out_p++ = '?';
402408
}
403409
else
404410
{
@@ -408,10 +414,11 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
408414
return 0;
409415
}
410416
++pindex;
411-
strncat(sql_out, start, p - start);
417+
memcpy(sql_out_p, start, p - start);
418+
sql_out_p += p - start;
412419
}
413420
break;
414-
421+
}
415422
case ttIdent:
416423
if (execBlock)
417424
{
@@ -423,11 +430,14 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
423430
if (l > 252) {
424431
return 0;
425432
}
426-
strncpy(ident, start, l);
433+
memcpy(ident, start, l);
427434
ident[l] = '\0';
428435
if (!strcasecmp(ident, "AS"))
429436
{
430-
strncat(sql_out, start, end - start);
437+
memcpy(sql_out_p, start, end - start);
438+
sql_out_p += end - start;
439+
*sql_out_p = '\0';
440+
*sql_out_len = sql_out_p - sql_out;
431441
return 1;
432442
}
433443
}
@@ -438,7 +448,8 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
438448
case ttComment:
439449
case ttString:
440450
case ttOther:
441-
strncat(sql_out, start, p - start);
451+
memcpy(sql_out_p, start, p - start);
452+
sql_out_p += p - start;
442453
break;
443454

444455
case ttBrokenComment:
@@ -455,6 +466,8 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa
455466
return 0;
456467
}
457468
}
469+
*sql_out_p = '\0';
470+
*sql_out_len = sql_out_p - sql_out;
458471
return 1;
459472
}
460473

@@ -789,7 +802,7 @@ static zend_long firebird_handle_doer(pdo_dbh_t *dbh, const zend_string *sql) /*
789802
static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
790803
{
791804
size_t qcount = 0;
792-
char const *co, *l, *r;
805+
char const *co, *l;
793806
char *c;
794807
size_t quotedlen;
795808
zend_string *quoted_str;
@@ -798,9 +811,15 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un
798811
return ZSTR_INIT_LITERAL("''", 0);
799812
}
800813

814+
const char * const end = ZSTR_VAL(unquoted) + ZSTR_LEN(unquoted);
815+
801816
/* Firebird only requires single quotes to be doubled if string lengths are used */
802817
/* count the number of ' characters */
803-
for (co = ZSTR_VAL(unquoted); (co = strchr(co,'\'')); qcount++, co++);
818+
for (co = ZSTR_VAL(unquoted); co < end; co++) {
819+
if (*co == '\'') {
820+
qcount++;
821+
}
822+
}
804823

805824
if (UNEXPECTED(ZSTR_LEN(unquoted) + 2 > ZSTR_MAX_LEN - qcount)) {
806825
return NULL;
@@ -812,15 +831,14 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un
812831
*c++ = '\'';
813832

814833
/* foreach (chunk that ends in a quote) */
815-
for (l = ZSTR_VAL(unquoted); (r = strchr(l,'\'')); l = r+1) {
816-
strncpy(c, l, r-l+1);
817-
c += (r-l+1);
818-
/* add the second quote */
819-
*c++ = '\'';
834+
for (l = ZSTR_VAL(unquoted); l < end; l++) {
835+
*c++ = *l;
836+
if (*l == '\'') {
837+
/* add the second quote */
838+
*c++ = '\'';
839+
}
820840
}
821841

822-
/* copy the remainder */
823-
strncpy(c, l, quotedlen-(c-ZSTR_VAL(quoted_str))-1);
824842
ZSTR_VAL(quoted_str)[quotedlen-1] = '\'';
825843
ZSTR_VAL(quoted_str)[quotedlen] = '\0';
826844

@@ -998,6 +1016,7 @@ static int php_firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const zend_string *sq
9981016
{
9991017
pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
10001018
char *new_sql;
1019+
size_t new_sql_len;
10011020

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

10181037
/* prepare the statement */
1019-
if (isc_dsql_prepare(H->isc_status, &H->tr, s, 0, new_sql, H->sql_dialect, out_sqlda)) {
1038+
if (isc_dsql_prepare(H->isc_status, &H->tr, s, new_sql_len, new_sql, H->sql_dialect, out_sqlda)) {
10201039
php_firebird_error(dbh);
10211040
efree(new_sql);
10221041
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)