--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--