#20467 [Ctl]: Buffer overflow returning binary

From: Date: Mon, 09 Dec 2002 10:11:37 +0000
Subject: #20467 [Ctl]: Buffer overflow returning binary
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-27143@lists.php.net to get a copy of this message
 ID:               20467
 User updated by:  freddy77@angelfire.com
 Reported By:      freddy77@angelfire.com
 Status:           Critical
 Bug Type:         Sybase (dblib) related
 Operating System: Linux
 PHP Version:      4.2.3
 New Comment:

You have also to decide what result you want for binary data.
If you use dbconvert for this type your results will be a hexadecimal
string. If you want binary data instead you should manually copy data
(use just memcpy) or do a conversion to binary, not to characters.

freddy77


Previous Comments:
------------------------------------------------------------------------

[2002-12-07 01:29:42] iliaa@php.net

Someone familiar with the workings of the sybase extension should
definately review the patch and make the necessary corrections before
the next RC is out.

------------------------------------------------------------------------

[2002-11-18 13:25:50] freddy77@angelfire.com

I found also another problem.
If NULL is returned (length == 0) the loop 
"while (*p == ' ') --p;"
can lead to a buffer underflow,
"while (p >= res_buf && *p == ' ') --p;"
fix the problem

freddy77

------------------------------------------------------------------------

[2002-11-17 13:34:54] freddy77@angelfire.com

I forgot. To check the issue try:

#!/home/freddy/install/bin/php -q
<?php
$db_handle = sybase_connect("server","user","pass");
sybase_select_db("tempdb", $db_handle);
$db_result = sybase_query("select convert(varbinary,space(120))");
$res = sybase_fetch_row($db_result);
print "\nres=$res[0]\n";
print sybase_num_rows($db_result);
$db_result = sybase_query("select convert(numeric(18,2),1234.89)");
$res = sybase_fetch_row($db_result);
print "\nres=$res[0]\n";
print sybase_num_rows($db_result);
?>

freddy77

------------------------------------------------------------------------

[2002-11-17 13:31:00] freddy77@angelfire.com

dbconvert convert binary -> char returning binary representation so
0x6789 (2 bytes) became '6789' (4 single byte characters)
When converting back to PHP (in ext/sybase/php_sybase_db.c) you pass
the same size buffer leading to a buffer overflow.

Following patch fix problem. It also fix another problem (it remove
last characters from conversion) and avoid future possible buffer
overflows due to strange types (like UNIQUEIDs in MSSQL)

diff -r -u10 php-4.2.3/ext/sybase/php_sybase_db.c
php-4.2.3mod/ext/sybase/php_sybase_db.c
--- php-4.2.3/ext/sybase/php_sybase_db.c	Wed Mar  6 16:59:42 2002
+++ php-4.2.3mod/ext/sybase/php_sybase_db.c	Sun Nov 17 20:08:31 2002
@@ -710,49 +710,51 @@
 		/*case SYBFLT8:*/
 		case SYBREAL: {
 			Z_DVAL_P(result) = (double) floatcol(offset);
 			Z_TYPE_P(result) = IS_DOUBLE;
 			break;
 		}
 		default: {
 			if (dbwillconvert(coltype(offset),SYBCHAR)) {
 				char *res_buf;
 				int res_length = dbdatlen(sybase_ptr->link,offset);
+				int src_length = res_length;
 				register char *p;
 			
 				switch (coltype(offset)) {
 					case SYBBINARY:
 					case SYBVARBINARY:
+						res_length *= 2;
+						break;
 					case SYBCHAR:
 					case SYBVARCHAR:
 					case SYBTEXT:
 					case SYBIMAGE:
 						break;
 					default:
 						/* take no chances, no telling how big the result would really
be */
 						res_length += 20;
 						break;
 				}
 
 				res_buf = (char *) emalloc(res_length+1);
 				memset(res_buf,' ',res_length+1);  /* XXX i'm sure there's a
better way
 										
        		  but i don't have sybase here to test
 										
        		  991105 thies@thieso.net  */
-				dbconvert(NULL,coltype(offset),dbdata(sybase_ptr->link,offset),
res_length,SYBCHAR,res_buf,-1);
+				dbconvert(NULL,coltype(offset),dbdata(sybase_ptr->link,offset),
src_length,SYBCHAR,res_buf,res_length);
 		
 				/* get rid of trailing spaces */
 				p = res_buf + res_length;
-				while (*p == ' ') {
+				while (*p == ' ')
 					p--;
-					res_length--;
-				}
 				*(++p) = 0; /* put a trailing NULL */
+				res_length = p - res_buf;
 		
 				Z_STRLEN_P(result) = res_length;
 				Z_STRVAL_P(result) = res_buf;
 				Z_TYPE_P(result) = IS_STRING;
 			} else {
 				php_error(E_WARNING,"Sybase:  column %d has unknown data type
(%d)", offset, coltype(offset));
 				ZVAL_FALSE(result);
 			}
 		}
 	}

Frediano Ziglio



------------------------------------------------------------------------


-- 
Edit this bug report at http://bugs.php.net/?id=20467&edit=1



Thread (10 messages)

« previous php.bugs (#27143) next »