Fix phpGH-22668: odbc heap over-read on oversized column value

ext/odbc bound a displaysize+1 buffer per column but built the result
string from the driver-reported vallen. A conforming driver truncates an
over-long value into the buffer yet reports the full length, so the fetch
over-read past the allocation and returned adjacent heap bytes to userland.
Store the bound capacity in odbc_result_value and clamp the string length to
it at the three bound-buffer fetch sites (odbc_fetch_array/object,
odbc_fetch_into, odbc_result); the SQLGetData long-column paths stay bounded
by longreadlen.

Fixes phpGH-22668
Closes phpGH-22670

Author: iliaal

NEWS

@@ -38,6 +38,10 @@ PHP                                                                        NEWS
   . Fixed IntlChar methods leaving stale global error state after successful
     calls. (Xuyang Zhang)
 
+- ODBC:
+  . Fixed bug GH-22668 (Heap buffer over-read when a column value exceeds the
+    driver-reported display size). (iliaal)
+
 - OpenSSL:
   . Fixed timeout for supplemental read at end of a blocking stream in SSL
     stream wrapper. (ilutov)

ext/odbc/php_odbc.c

@@ -680,6 +680,7 @@ void odbc_bindcols(odbc_result *result)
 
 	for(i = 0; i < result->numcols; i++) {
 		charextraalloc = 0;
+		result->values[i].value_max_len = 0;
 		colfieldid = SQL_COLUMN_DISPLAY_SIZE;
 
 		rc = PHP_ODBC_SQLCOLATTRIBUTE(result->stmt, (SQLUSMALLINT)(i+1), PHP_ODBC_SQL_DESC_NAME,
@@ -706,6 +707,7 @@ void odbc_bindcols(odbc_result *result)
 #ifdef HAVE_ADABAS
 			case SQL_TIMESTAMP:
 				result->values[i].value = (char *)emalloc(27);
+				result->values[i].value_max_len = 26;
 				SQLBindCol(result->stmt, (SQLUSMALLINT)(i+1), SQL_C_CHAR, result->values[i].value,
 							27, &result->values[i].vallen);
 				break;
@@ -775,6 +777,7 @@ void odbc_bindcols(odbc_result *result)
 					displaysize *= 4;
 				}
 				result->values[i].value = (char *)emalloc(displaysize + 1);
+				result->values[i].value_max_len = displaysize;
 				rc = SQLBindCol(result->stmt, (SQLUSMALLINT)(i+1), SQL_C_CHAR, result->values[i].value,
 							displaysize + 1, &result->values[i].vallen);
 				break;
@@ -1492,7 +1495,11 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
 					ZVAL_FALSE(&tmp);
 					break;
 				}
-				ZVAL_STRINGL(&tmp, result->values[i].value, result->values[i].vallen);
+				SQLLEN str_len = result->values[i].vallen;
+				if (str_len > result->values[i].value_max_len) {
+					str_len = result->values[i].value_max_len;
+				}
+				ZVAL_STRINGL(&tmp, result->values[i].value, str_len);
 				break;
 		}
 
@@ -1660,7 +1667,11 @@ PHP_FUNCTION(odbc_fetch_into)
 					ZVAL_FALSE(&tmp);
 					break;
 				}
-				ZVAL_STRINGL(&tmp, result->values[i].value, result->values[i].vallen);
+				SQLLEN str_len = result->values[i].vallen;
+				if (str_len > result->values[i].value_max_len) {
+					str_len = result->values[i].value_max_len;
+				}
+				ZVAL_STRINGL(&tmp, result->values[i].value, str_len);
 				break;
 		}
 		zend_hash_index_update(Z_ARRVAL_P(pv_res_arr), i, &tmp);
@@ -1910,7 +1921,11 @@ PHP_FUNCTION(odbc_result)
 				php_error_docref(NULL, E_WARNING, "Cannot get data of column #%d (driver cannot determine length)", field_ind + 1);
 				RETURN_FALSE;
 			} else {
-				RETURN_STRINGL(result->values[field_ind].value, result->values[field_ind].vallen);
+				SQLLEN str_len = result->values[field_ind].vallen;
+				if (str_len > result->values[field_ind].value_max_len) {
+					str_len = result->values[field_ind].value_max_len;
+				}
+				RETURN_STRINGL(result->values[field_ind].value, str_len);
 			}
 			break;
 	}

ext/odbc/php_odbc_includes.h

@@ -205,6 +205,7 @@ typedef struct odbc_result_value {
 	char name[256];
 	char *value;
 	SQLLEN vallen;
+	SQLLEN value_max_len;
 	SQLLEN coltype;
 } odbc_result_value;
 

ext/odbc/tests/config.inc

@@ -1,5 +1,7 @@
 <?php
 
+defined('ODBC_SQLITE_DSN') || define('ODBC_SQLITE_DSN', "Driver=SQLite3;Database=:memory:");
+
 $dsn  = getenv("ODBC_TEST_DSN");
 $user = getenv("ODBC_TEST_USER");
 $pass = getenv("ODBC_TEST_PASS");

ext/odbc/tests/gh22668.phpt

@@ -0,0 +1,33 @@
+--TEST--
+GH-22668 (Heap buffer over-read when a column value exceeds the bound buffer)
+--EXTENSIONS--
+odbc
+--SKIPIF--
+<?php
+include __DIR__ . "/config.inc";
+$conn = @odbc_connect(ODBC_SQLITE_DSN, null, null);
+if (!$conn) {
+    die("skip requires the SQLite3 ODBC driver");
+}
+?>
+--FILE--
+<?php
+include __DIR__ . "/config.inc";
+$conn = odbc_connect(ODBC_SQLITE_DSN, null, null);
+
+// The SQLite3 driver reports a 255 byte display size for a computed column, so
+// the bound buffer holds at most 255 bytes while the value is far longer. A
+// conforming driver truncates into the buffer but reports the full length; the
+// returned string must stay within the buffer, not over-read past it.
+$result = odbc_exec($conn, "SELECT printf('%.*c', 4096, 'A') AS data");
+$row = odbc_fetch_array($result);
+$s = $row['data'];
+
+echo "clamped to buffer: "; var_dump(strlen($s) < 4096);
+echo "only value bytes:  "; var_dump(strlen($s) === substr_count($s, 'A'));
+
+odbc_close($conn);
+?>
+--EXPECT--
+clamped to buffer: bool(true)
+only value bytes:  bool(true)