php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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:
 [2012-10-08 07:36 UTC] remi@php.net
Description:
------------
Already report on internals http://marc.info/?t=134262688600006&r=1&w=2

Discard state is 5 char long, so buffer must be 6.

Trivial fix attached.
(could apply in all branches)



Patches

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

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-10-08 07:37 UTC] remi@php.net
The following patch has been added/updated:

Patch Name: php-5.3.3-pdo-overflow.patch
Revision:   1349681821
URL:        https://bugs.php.net/patch-display.php?bug=63235&patch=php-5.3.3-pdo-overflow.patch&revision=1349681821
 [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)
 [2012-10-09 05:13 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=45e0d452c5c369f0141fde780a6cbdd35d8f55b4
Log: Fixed bug #63235 (buffer overflow in use of SQLGetDiagRec)
 [2012-10-09 05:13 UTC] laruence@php.net
-Status: Open +Status: Closed
 [2012-10-09 05:15 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=45e0d452c5c369f0141fde780a6cbdd35d8f55b4
Log: Fixed bug #63235 (buffer overflow in use of SQLGetDiagRec)
 [2012-10-09 05:16 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=45e0d452c5c369f0141fde780a6cbdd35d8f55b4
Log: Fixed bug #63235 (buffer overflow in use of SQLGetDiagRec)
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Mon Apr 21 10:02:10 2014 UTC