dbTalk Databases Forums  

[BUGS] Fix for buffer overflow ready [was: Fwd: Bug#247306: odbc-postgresql: SIGSEGV with long inputs (> 10000 bytes)]

mailing.database.pgsql-bugs mailing.database.pgsql-bugs


Discuss [BUGS] Fix for buffer overflow ready [was: Fwd: Bug#247306: odbc-postgresql: SIGSEGV with long inputs (> 10000 bytes)] in the mailing.database.pgsql-bugs forum.



Reply
 
Thread Tools Display Modes
  #1  
Old   
Martin Pitt
 
Posts: n/a

Default [BUGS] Fix for buffer overflow ready [was: Fwd: Bug#247306: odbc-postgresql: SIGSEGV with long inputs (> 10000 bytes)] - 05-17-2004 , 02:05 PM






--c3bfwLpm8qysLVxt
Content-Type: multipart/mixed; boundary="E39vaYmALEf/7YXx"
Content-Disposition: inline

--E39vaYmALEf/7YXx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi again!

Sorry for crossposting, but I sent the initial post also to -bugs,
because I did not get an answer on -odbc.

On 2004-05-11 12:03 +0200, Martin Pitt wrote:
Quote:
I noticed Apache segfaulting when I feed a simple form with long inputs:
=20
[Tue May 4 11:32:10 2004] [notice] child pid 4084 exit signal Segmentat=
ion fault (11)
=20
Such inputs are used by php function odbc_connect as username and passwor=
d to connect to a DSN using postgresql driver:
=20
$connection =3D @odbc_connect(DSN, $_POST['username'], $_POST['password'=
])
=20
The output of gdb is:
=20
(gdb) run -X -d apache
[...]
[Thread debugging using libthread_db enabled]
[...]
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1076569920 (LWP 832)]
0x44c3d627 in SOCK_put_next_byte () from /usr/lib/postgresql/lib/psqlodb=
c.so
=20
Or:
[same stuff here]
0x44c4c3d0 in strncpy_null () from /usr/lib/postgresql/lib/psqlodbc.so
=20
I suspect a security issue because playing around with long input strings=
of "A" I've been able to trigger in Apache error.log this message:
=20=09
free(): invalid pointer 0x41414141!
=20
0x41 is obviously one of my "A"...
The problem is that make_string() in misc.c does not check whether the
target buffer is big enough to hold the copied string.

I added a bufsize parameter to make_string() and used it in all calls
to it. I tried it with my php4 crash test script and now it works
properly.

The attached patch is for the current stable release 07.03.0200.

Thanks a lot to Peter Eisentraut for pointing me at the problem origin.

Unless you have a better idea it would be nice if you could apply the
patch to the official sources and also include it in the next release.

I will upload updated Debian packages for unstable and stable this
afternoon (16:00 CEST) if nobody reports a problem or a better
solution.

Thanks in advance,

Martin

--=20
Martin Pitt Debian GNU/Linux Developer
martin (AT) piware (DOT) de mpitt (AT) debian (DOT) org
http://www.piware.de http://www.debian.org

--E39vaYmALEf/7YXx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="psqlodbc-make_string.patch"
Content-Transfer-Encoding: quoted-printable

Index: connection.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D
RCS file: /cvsroot/pkg-postgresql/psqlodbc/psqlodbc-07.03.0200/connection.c=
,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 connection.c
--- connection.c 22 Jan 2004 15:02:52 -0000 1.1.1.1
+++ connection.c 13 May 2004 08:47:22 -0000
@@ -107,7 +107,7 @@
=20
ci =3D &conn->connInfo;
=20
- make_string(szDSN, cbDSN, ci->dsn);
+ make_string(szDSN, cbDSN, ci->dsn, sizeof(ci->dsn));
=20
/* get the values for the DSN from the registry */
memcpy(&ci->drivers, &globals, sizeof(globals));
@@ -120,8 +120,8 @@
* override values from DSN info with UID and authStr(pwd) This only
* occurs if the values are actually there.
*/
- make_string(szUID, cbUID, ci->username);
- make_string(szAuthStr, cbAuthStr, ci->password);
+ make_string(szUID, cbUID, ci->username,sizeof(ci->username));
+ make_string(szAuthStr, cbAuthStr, ci->password, sizeof(ci->password));
=20
/* fill in any defaults */
getDSNdefaults(ci);
Index: drvconn.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D
RCS file: /cvsroot/pkg-postgresql/psqlodbc/psqlodbc-07.03.0200/drvconn.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 drvconn.c
--- drvconn.c 22 Jan 2004 15:02:52 -0000 1.1.1.1
+++ drvconn.c 13 May 2004 08:47:22 -0000
@@ -112,7 +112,7 @@
return SQL_INVALID_HANDLE;
}
=20
- make_string(szConnStrIn, cbConnStrIn, connStrIn);
+ make_string(szConnStrIn, cbConnStrIn, connStrIn, sizeof(connStrIn));
=20
#ifdef FORCE_PASSWORD_DISPLAY
mylog("**** PGAPI_DriverConnect: fDriverCompletion=3D%d, connStrIn=3D'%s'=
\n", fDriverCompletion, connStrIn);
Index: execute.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D
RCS file: /cvsroot/pkg-postgresql/psqlodbc/psqlodbc-07.03.0200/execute.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 execute.c
--- execute.c 22 Jan 2004 15:02:49 -0000 1.1.1.1
+++ execute.c 13 May 2004 08:47:22 -0000
@@ -101,7 +101,7 @@
if (!szSqlStr[0])
self->statement =3D strdup("");
else
- self->statement =3D make_string(szSqlStr, cbSqlStr, NULL);
+ self->statement =3D make_string(szSqlStr, cbSqlStr, NULL, 0);
if (!self->statement)
{
SC_set_error(self, STMT_NO_MEMORY_ERROR, "No memory available to store s=
tatement");
@@ -150,7 +150,7 @@
* keep a copy of the un-parametrized statement, in case they try to
* execute this statement again
*/
- stmt->statement =3D make_string(szSqlStr, cbSqlStr, NULL);
+ stmt->statement =3D make_string(szSqlStr, cbSqlStr, NULL, 0);
if (!stmt->statement)
{
SC_set_error(stmt, STMT_NO_MEMORY_ERROR, "No memory available to store s=
tatement");
@@ -775,7 +775,7 @@
=20
mylog("%s: entering...cbSqlStrIn=3D%d\n", func, cbSqlStrIn);
=20
- ptr =3D (cbSqlStrIn =3D=3D 0) ? "" : make_string(szSqlStrIn, cbSqlStrIn, =
NULL);
+ ptr =3D (cbSqlStrIn =3D=3D 0) ? "" : make_string(szSqlStrIn, cbSqlStrIn, =
NULL, 0);
if (!ptr)
{
CC_set_error(conn, CONN_NO_MEMORY_ERROR, "No memory available to store n=
ative sql string");
Index: info.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D
RCS file: /cvsroot/pkg-postgresql/psqlodbc/psqlodbc-07.03.0200/info.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 info.c
--- info.c 22 Jan 2004 15:02:50 -0000 1.1.1.1
+++ info.c 13 May 2004 08:47:23 -0000
@@ -1387,7 +1387,7 @@
show_views =3D FALSE;
=20
/* make_string mallocs memory */
- tableType =3D make_string(szTableType, cbTableType, NULL);
+ tableType =3D make_string(szTableType, cbTableType, NULL, 0);
if (tableType)
{
strcpy(table_types, tableType);
@@ -2555,7 +2555,7 @@
* only use the table name... the owner should be redundant, and we
* never use qualifiers.
*/
- table_name =3D make_string(szTableName, cbTableName, NULL);
+ table_name =3D make_string(szTableName, cbTableName, NULL, 0);
if (!table_name)
{
SC_set_error(stmt, STMT_INTERNAL_ERROR, "No table name passed to PGAPI_S=
tatistics.");
@@ -3009,7 +3009,7 @@
internal_asis_type =3D INTERNAL_ASIS_TYPE;
#endif /* UNICODE_SUPPORT */
pktab[0] =3D '\0';
- make_string(szTableName, cbTableName, pktab);
+ make_string(szTableName, cbTableName, pktab, sizeof(pktab));
if (pktab[0] =3D=3D '\0')
{
SC_set_error(stmt, STMT_INTERNAL_ERROR, "No Table specified to PGAPI_Pri=
maryKeys.");
@@ -3543,8 +3543,8 @@
schema_needed[0] =3D '\0';
schema_fetched[0] =3D '\0';
=20
- make_string(szPkTableName, cbPkTableName, pk_table_needed);
- make_string(szFkTableName, cbFkTableName, fk_table_needed);
+ make_string(szPkTableName, cbPkTableName, pk_table_needed, sizeof(pk_tabl=
e_needed));
+ make_string(szFkTableName, cbFkTableName, fk_table_needed, sizeof(fk_tabl=
e_needed));
=20
conn =3D SC_get_conn(stmt);
#ifdef UNICODE_SUPPORT
Index: misc.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D
RCS file: /cvsroot/pkg-postgresql/psqlodbc/psqlodbc-07.03.0200/misc.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 misc.c
--- misc.c 22 Jan 2004 15:02:50 -0000 1.1.1.1
+++ misc.c 13 May 2004 08:47:23 -0000
@@ -259,12 +259,13 @@
/*------
* Create a null terminated string (handling the SQL_NTS thing):
* 1. If buf is supplied, place the string in there
- * (assumes enough space) and return buf.
- * 2. If buf is not supplied, malloc space and return this string
+ * (at most bufsize-1 bytes) and return buf.
+ * 2. If buf is not supplied, malloc space and return this string;
+ * (buflen is ignored in this case).
*------
*/
char *
-make_string(const char *s, int len, char *buf)
+make_string(const char *s, int len, char *buf, int bufsize)
{
int length;
char *str;
@@ -275,6 +276,8 @@
=20
if (buf)
{
+ if(length >=3D bufsize)
+ length =3D bufsize-1;
strncpy_null(buf, s, length + 1);
return buf;
}
Index: misc.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D
RCS file: /cvsroot/pkg-postgresql/psqlodbc/psqlodbc-07.03.0200/misc.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 misc.h
--- misc.h 22 Jan 2004 15:02:50 -0000 1.1.1.1
+++ misc.h 13 May 2004 08:47:23 -0000
@@ -119,7 +119,7 @@
void remove_newlines(char *string);
char *strncpy_null(char *dst, const char *src, int len);
char *trim(char *string);
-char *make_string(const char *s, int len, char *buf);
+char *make_string(const char *s, int len, char *buf, int bufsize );
char *make_lstring_ifneeded(ConnectionClass *, const char *s, int len, =
BOOL);
char *my_strcat(char *buf, const char *fmt, const char *s, int len);
char *schema_strcat(char *buf, const char *fmt, const char *s, int len,

--E39vaYmALEf/7YXx--

--c3bfwLpm8qysLVxt
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQFAo0LiDecnbV4Fd/IRAh69AJ4wheJEQ9TVXvaRe2FmyVmDzw4YHQCfV21K
1M0CShpn4xYKF6Y0P6ELYYI=
=LSnq
-----END PGP SIGNATURE-----

--c3bfwLpm8qysLVxt--


Reply With Quote
Reply




Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off



Powered by vBulletin Version 3.5.3
Copyright ©2000 - 2012, Jelsoft Enterprises Ltd.