Sprintf() auditing and a patch - Mailing list pgsql-hackers

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