Skip to content

Commit aae797d

Browse files
committed
Fix GH-22667: pdo_odbc heap over-read on oversized column value
odbc_stmt_describe() binds a colsize+1 buffer for short columns and stores that capacity in datalen, but odbc_stmt_get_col() built the result string from the driver-reported fetched_len. A conforming driver truncates an over-long value into the buffer yet reports the full length, so ZVAL_STRINGL_FAST over-read past the allocation and returned adjacent heap to userland. Clamp fetched_len to the bound capacity, guarded by !is_long so the long-column SQLGetData path (whose fixed buffer is unrelated to datalen) is left untouched. Fixes GH-22667
1 parent ebed41a commit aae797d

3 files changed

Lines changed: 45 additions & 1 deletion

File tree

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ PHP NEWS
4343
- PDO_ODBC:
4444
. Fixed bug GH-20726 (Crash with ODBC connection pooling when the DSN
4545
carries no credentials). (iliaal)
46+
. Fixed bug GH-22667 (Heap buffer over-read when a column value exceeds the
47+
driver-reported display size). (iliaal)
4648

4749
- Phar:
4850
. Fixed inconsistent handling of the magic ".phar" directory. Paths such as

ext/pdo_odbc/odbc_stmt.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,14 @@ static int odbc_stmt_get_col(pdo_stmt_t *stmt, int colno, zval *result, enum pdo
744744
return 1;
745745
} else if (C->fetched_len >= 0) {
746746
/* it was stored perfectly */
747-
ZVAL_STRINGL_FAST(result, C->data, C->fetched_len);
747+
SQLLEN data_len = C->fetched_len;
748+
if (!C->is_long) {
749+
SQLLEN max_len = C->is_unicode ? (SQLLEN)C->datalen + 1 : (SQLLEN)C->datalen;
750+
if (data_len > max_len) {
751+
data_len = max_len;
752+
}
753+
}
754+
ZVAL_STRINGL_FAST(result, C->data, data_len);
748755
if (C->is_unicode) {
749756
goto unicode_conv;
750757
}

ext/pdo_odbc/tests/gh22667.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
GH-22667 (Heap buffer over-read when a column value exceeds the bound buffer)
3+
--EXTENSIONS--
4+
pdo_odbc
5+
--SKIPIF--
6+
<?php
7+
$file = tempnam(sys_get_temp_dir(), "gh22667");
8+
try {
9+
$pdo = new PDO("odbc:Driver=SQLite3;Database=$file");
10+
} catch (PDOException $e) {
11+
@unlink($file);
12+
die("skip requires the SQLite3 ODBC driver");
13+
}
14+
?>
15+
--FILE--
16+
<?php
17+
$file = tempnam(sys_get_temp_dir(), "gh22667");
18+
$pdo = new PDO("odbc:Driver=SQLite3;Database=$file");
19+
20+
// The SQLite3 driver reports a 255 byte display size for a computed column, so
21+
// the short-bound buffer holds at most 255 bytes while the value is far longer.
22+
// A conforming driver truncates into the buffer but reports the full length; the
23+
// returned string must stay within the buffer, not over-read past it.
24+
$stmt = $pdo->query("SELECT printf('%.*c', 4096, 'A') AS data");
25+
$row = $stmt->fetch(PDO::FETCH_ASSOC);
26+
$s = $row['data'];
27+
28+
echo "clamped to buffer: "; var_dump(strlen($s) < 4096);
29+
echo "only value bytes: "; var_dump(strlen($s) === substr_count($s, 'A'));
30+
31+
@unlink($file);
32+
?>
33+
--EXPECT--
34+
clamped to buffer: bool(true)
35+
only value bytes: bool(true)

0 commit comments

Comments
 (0)