Sprintf() auditing and a patch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

I'm very new to this project and inspired by recent security release, I
started to audit postgresql source against common mistakes with sprintf().

I mostly found problems with sprintf() used on statically allocated
buffers or dynamically allocated buffers with random constant size.

I used lib/stringinfo.h functions when I was sure palloc()-memory
allocation was the right thing to do and I felt like code needed to
construct a complete string no matter how complex.

There were places where I just changes sprintf() to snprintf(). Like in
some *BSD dl loading functions etc.

There were also places where I could identify the possible bug but
didn't know 'the' right way to fix it. As I say, I don't know the
codebase very well so I really didn't know what auxiliarity functions
there are to use. These parts are marked as FIXME and should be easily
identified by looking at the patch (link below - it is a big one).

There were also simple mistakes like in src/backend/tioga/tgRecipe.c
- - sprintf(qbuf, Q_LOOKUP_EDGES_IN_RECIPE, name);
- - pqres = PQexec(qbuf);
+ snprintf(qbuf, MAX_QBUF_LENGTH, Q_LOOKUP_EDGES_IN_RECIPE, name);
~ pqres = PQexec(qbuf);
~ if (*pqres == 'R' || *pqres == 'E')

Notice how previous PQexec() is removed. There were two of them.

Some of my fixes cause code to be a bit slower because of dynamically
allocated mem, but it also fixes a lot of ptr+strlen(ptr) -style
performance problems. I didn't particularly try to fix these but some of
them are corrected by simply using lib/stringinfo.h

Please take look at this patch but since I have worked three long nights
with this one, there probably are bugs. I tried compiling it with
"configure --with-tcl --with-perl --with-python" and at least it
compiled for me :) But that's about all I can promise.

diffstat postgresql-7.2.2-sprintf.patch
~ contrib/cube/cube.c | 26 --
~ contrib/cube/cubeparse.y | 11
~ contrib/intarray/_int.c | 29 +-
~ contrib/rserv/rserv.c | 30 +-
~ contrib/seg/segparse.y | 18 -
~ contrib/spi/refint.c | 39 +--
~ contrib/spi/timetravel.c | 12
~ doc/src/sgml/spi.sgml | 2
~ src/backend/parser/analyze.c | 2
~ src/backend/port/dynloader/freebsd.c | 10
~ src/backend/port/dynloader/netbsd.c | 11
~ src/backend/port/dynloader/nextstep.c | 2
~ src/backend/port/dynloader/openbsd.c | 10
~ src/backend/postmaster/postmaster.c | 2
~ src/backend/storage/file/fd.c | 1
~ src/backend/storage/ipc/shmqueue.c | 1
~ src/backend/tioga/tgRecipe.c | 11
~ src/backend/utils/adt/ri_triggers.c | 312
++++++++++++------------
~ src/bin/pg_dump/pg_dump.c | 14 -
~ src/bin/pg_passwd/pg_passwd.c | 2
~ src/bin/psql/command.c | 2
~ src/bin/psql/describe.c | 3
~ src/interfaces/ecpg/preproc/pgc.l | 8
~ src/interfaces/ecpg/preproc/preproc.y | 24 -
~ src/interfaces/ecpg/preproc/type.c | 16 -
~ src/interfaces/ecpg/preproc/variable.c | 12
~ src/interfaces/libpgeasy/examples/pgwordcount.c | 6
~ src/interfaces/libpgtcl/pgtclCmds.c | 4
~ src/interfaces/libpq/fe-auth.c | 2
~ src/interfaces/odbc/connection.c | 2
~ src/interfaces/odbc/dlg_specific.c | 5
~ src/interfaces/odbc/info.c | 38 +-
~ src/interfaces/odbc/qresult.c | 4
~ src/interfaces/odbc/results.c | 8
~ src/interfaces/odbc/statement.c | 6
~ 35 files changed, 365 insertions, 320 deletions

Patch is about 70k and downloadable from
http://suihkari.baana.suomi.net/postgresql/patches/postgresql-7.2.2-sprintf.patch

At least I didn't just bitch and moan about the bugs. ;)

- - Jukka
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE9bRZSYYWM2XTSwX0RApcSAJ40pTB0DEiucS/4m2aNFHSn5XVXlwCfeyYT
EL5AF82ZlcqT/dGgd6BRJWM=
=qojm
-----END PGP SIGNATURE-----