buffer overflow in use of SQLGetDiagRec

Bug #63235 buffer overflow in use of SQLGetDiagRec
Submitted: 2012-10-08 07:36 UTC Modified: 2012-10-09 04:58 UTC
From: remi@php.net Assigned:
Status: Closed Package: PDO related
PHP Version: 5.4.7 OS: GNU/Linux
Private report: No CVE-ID: None

Patches

php-5.3.3-pdo-overflow.patch (last revision 2012-10-08 07:37 UTC by remi@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

 [2012-10-08 15:35 UTC] laruence@php.net

maybe the discard_buf should also be consistent with struct 
pdo_odbc_errinfo.last_err_msg 

which is "char last_err_msg[SQL_MAX_MESSAGE_LENGTH];"

diff is:

diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c
index 84a147b..2176051 100755
--- a/ext/pdo_odbc/odbc_driver.c
+++ b/ext/pdo_odbc/odbc_driver.c
@@ -114,8 +114,8 @@ void pdo_odbc_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, 
PDO_ODBC_HSTMT statement,
 	 * diagnostic records (which can be generated by PRINT statements
 	 * in the query, for instance). */
 	while (rc == SQL_SUCCESS || rc == SQL_SUCCESS_WITH_INFO) {
-		char discard_state[5];
-		char discard_buf[1024];
+		char discard_state[6];
+		char discard_buf[SQL_MAX_MESSAGE_LENGTH];
 		SQLINTEGER code;
 		rc = SQLGetDiagRec(htype, eh, recno++, discard_state, &code,
 				discard_buf, sizeof(discard_buf)-1, 
&errmsgsize);

 [2012-10-08 18:14 UTC] remi@php.net

@laruence, I agree, but is this case should rather be SQL_MAX_MESSAGE_LENGTH+1 as used in unixODBC source code.

But this have no risk as this is (mostly) protected by the buffer_length arg.

From extract_sql_error_rec function source code
(unixODBC-2.3.1/DriverManager/SQLGetDiagRec.c)

    if ( sqlstate )
        strcpy((char*) sqlstate, "00000" );

Here is the buffer overflow issue (no length protection).

 [2012-10-09 02:12 UTC] laruence@php.net

yeah, I think you can commit that patch. thanks

 [2012-10-09 04:58 UTC] remi@php.net

@laruence: thanks, but I don't have commit right on php-src (only on pecl)