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: > I noticed Apache segfaulting when I feed a simple form with long inputs: > > [Tue May 4 11:32:10 2004] [notice] child pid 4084 exit signal Segmentation > fault (11) > > Such inputs are used by php function odbc_connect as username and password to > connect to a DSN using postgresql driver: > > $connection = @odbc_connect(DSN, $_POST['username'], $_POST['password']) > > The output of gdb is: > > (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/psqlodbc.so > > Or: > [same stuff here] > 0x44c4c3d0 in strncpy_null () from /usr/lib/postgresql/lib/psqlodbc.so > > 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: > > free(): invalid pointer 0x41414141! > > 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 -- Martin Pitt Debian GNU/Linux Developer [EMAIL PROTECTED] [EMAIL PROTECTED] http://www.piware.de http://www.debian.org
Index: connection.c
===================================================================
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 @@
ci = &conn->connInfo;
- make_string(szDSN, cbDSN, ci->dsn);
+ make_string(szDSN, cbDSN, ci->dsn, sizeof(ci->dsn));
/* 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));
/* fill in any defaults */
getDSNdefaults(ci);
Index: drvconn.c
===================================================================
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;
}
- make_string(szConnStrIn, cbConnStrIn, connStrIn);
+ make_string(szConnStrIn, cbConnStrIn, connStrIn, sizeof(connStrIn));
#ifdef FORCE_PASSWORD_DISPLAY
mylog("**** PGAPI_DriverConnect: fDriverCompletion=%d, connStrIn='%s'\n",
fDriverCompletion, connStrIn);
Index: execute.c
===================================================================
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 = strdup("");
else
- self->statement = make_string(szSqlStr, cbSqlStr, NULL);
+ self->statement = make_string(szSqlStr, cbSqlStr, NULL, 0);
if (!self->statement)
{
SC_set_error(self, STMT_NO_MEMORY_ERROR, "No memory available to store
statement");
@@ -150,7 +150,7 @@
* keep a copy of the un-parametrized statement, in case they try to
* execute this statement again
*/
- stmt->statement = make_string(szSqlStr, cbSqlStr, NULL);
+ stmt->statement = make_string(szSqlStr, cbSqlStr, NULL, 0);
if (!stmt->statement)
{
SC_set_error(stmt, STMT_NO_MEMORY_ERROR, "No memory available to store
statement");
@@ -775,7 +775,7 @@
mylog("%s: entering...cbSqlStrIn=%d\n", func, cbSqlStrIn);
- ptr = (cbSqlStrIn == 0) ? "" : make_string(szSqlStrIn, cbSqlStrIn, NULL);
+ ptr = (cbSqlStrIn == 0) ? "" : make_string(szSqlStrIn, cbSqlStrIn, NULL, 0);
if (!ptr)
{
CC_set_error(conn, CONN_NO_MEMORY_ERROR, "No memory available to store
native sql string");
Index: info.c
===================================================================
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 = FALSE;
/* make_string mallocs memory */
- tableType = make_string(szTableType, cbTableType, NULL);
+ tableType = 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 = make_string(szTableName, cbTableName, NULL);
+ table_name = make_string(szTableName, cbTableName, NULL, 0);
if (!table_name)
{
SC_set_error(stmt, STMT_INTERNAL_ERROR, "No table name passed to
PGAPI_Statistics.");
@@ -3009,7 +3009,7 @@
internal_asis_type = INTERNAL_ASIS_TYPE;
#endif /* UNICODE_SUPPORT */
pktab[0] = '\0';
- make_string(szTableName, cbTableName, pktab);
+ make_string(szTableName, cbTableName, pktab, sizeof(pktab));
if (pktab[0] == '\0')
{
SC_set_error(stmt, STMT_INTERNAL_ERROR, "No Table specified to
PGAPI_PrimaryKeys.");
@@ -3543,8 +3543,8 @@
schema_needed[0] = '\0';
schema_fetched[0] = '\0';
- make_string(szPkTableName, cbPkTableName, pk_table_needed);
- make_string(szFkTableName, cbFkTableName, fk_table_needed);
+ make_string(szPkTableName, cbPkTableName, pk_table_needed,
sizeof(pk_table_needed));
+ make_string(szFkTableName, cbFkTableName, fk_table_needed,
sizeof(fk_table_needed));
conn = SC_get_conn(stmt);
#ifdef UNICODE_SUPPORT
Index: misc.c
===================================================================
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 @@
if (buf)
{
+ if(length >= bufsize)
+ length = bufsize-1;
strncpy_null(buf, s, length + 1);
return buf;
}
Index: misc.h
===================================================================
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,
pgpbQaXkwQIwg.pgp
Description: PGP signature
