Patch for 1-byte buffer overflow in libpq PQencryptPassword

From: ljb <ljb1813(at)pobox(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Patch for 1-byte buffer overflow in libpq PQencryptPassword
Date: 2009-09-15 00:28:02
Message-ID: h8mn2i$1son$1@news.hub.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

A trivial little fix for PostgreSQL-8.4.1.

Calling the libpq function PQencryptPassword(password, "") doesn't make
a lot of sense (empty string for username). But if you do, it results
in a 1-byte buffer overflow in pg_md5_encrypt(). (This is in
backend/libpq/md5.c, but it's client, not backend.)

This is because pg_md5_encrypt(password, salt, salt_len, buf) with
salt_len=0 allocates a buffer crypt_buf of size strlen(password), then
uses strcpy to copy the password in there. The null byte at the end of
the password overruns the end of the allocated buffer.

(Found during pgtclng testing, looking for the cause of an error
on WinXP only, which turned out to have nothing to do with this.)

Two possible suggested fixes to src/backend/libpq/md5.c, pg_md5_crypt():
1) Allocate crypt_buf to (passwd_len + 1 + salt_len)
2) Use memcpy(crypt_buf, passwd, passwd_len) not strcpy(crypt_buf, passwd).

I like fix #2 better, although fix #1 avoids a weirdness with
PQencryptPassword("","") calling malloc(0) with platform-dependent
results (which was the problem I was chasing with pgtclng).

Patch below is for fix #2.

--- postgresql-8.4.1/src/backend/libpq/md5.c.bak 2009-01-01 12:23:42.000000000 -0500
+++ postgresql-8.4.1/src/backend/libpq/md5.c 2009-09-13 11:21:59.000000000 -0400
@@ -324,7 +324,7 @@
* Place salt at the end because it may be known by users trying to crack
* the MD5 output.
*/
- strcpy(crypt_buf, passwd);
+ memcpy(crypt_buf, passwd, passwd_len);
memcpy(crypt_buf + passwd_len, salt, salt_len);

strcpy(buf, "md5");

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2009-09-15 00:35:23 Re: Feature Request: JSON input for hstore
Previous Message Robert Haas 2009-09-14 23:45:46 Re: Feature Request: JSON input for hstore