Thread: BUG #3571: call to decrypt causes segfault

The following bug has been logged online:

Bug reference:      3571
Logged by:          Ken Colson
Email address:      ken.colson@sage.com
PostgreSQL version: 8.1.9
Operating system:   centOS 64 bit (Linux 2.6.18-8.1.6.el5)
Description:        call to decrypt causes segfault
Details:

this statement:

select decrypt(''::bytea,'password','bf')

causes the postgresql backend to crash:

kernel: postmaster[3368]: segfault at 000000011f391a83 rip 00002aaab2e6e1a0
rsp 00007fffdadd9be0 error 4

This seems to be a 64bit problem.  The problem was duplicated on 8.2.4 on 64
bit Ubuntu 7.

thanks,
Ken Colson

"Ken Colson" <ken.colson@sage.com> writes:
> this statement:
> select decrypt(''::bytea,'password','bf')
> causes the postgresql backend to crash:
> This seems to be a 64bit problem.

Reproduced here in HEAD.  The problem is here:

Program terminated with signal 11, Segmentation fault.
#0  0x00002aaaad2d41f0 in combo_decrypt (cx=0xb182f8,
    data=0xac991c "~\177\177\177ÀÙ¤", dlen=0, res=0xb1838c "~\177\177\177@¢±",
    rlen=0x7fffc1f499e4) at px.c:293
293                     pad = res[*rlen - 1];
(gdb) p res
$1 = (uint8 *) 0xb1838c "~\177\177\177@¢±"
(gdb) p rlen
$2 = (unsigned int *) 0x7fffc1f499e4
(gdb) p *rlen
$3 = 0

What apparently is happening is that the compiler chooses to interpret
"res[-1]" as "res[0xFFFFFFFF]".  On a 32-bit machine that wraps around
and you touch the previous byte, but on a 64-bit machine you touch
someplace in never-never land.

The problem clearly is that combo_decrypt()'s depadding code fails to
consider the possibility of a zero-length input, but I'm not entirely
sure how far up the food chain we ought to fix it --- perhaps
pg_decrypt() should not have bothered to light up the decryptor at all?
Also, what other pgcrypto routines might have similar bugs?

Marko, any time to work on this?

            regards, tom lane

On 8/23/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Ken Colson" <ken.colson@sage.com> writes:
> > this statement:
> > select decrypt(''::bytea,'password','bf')
> > causes the postgresql backend to crash:
> > This seems to be a 64bit problem.
>
> Reproduced here in HEAD.  The problem is here:

> 293                     pad = res[*rlen - 1];

> The problem clearly is that combo_decrypt()'s depadding code fails to
> consider the possibility of a zero-length input, but I'm not entirely
> sure how far up the food chain we ought to fix it --- perhaps
> pg_decrypt() should not have bothered to light up the decryptor at all?

The fix should be in combo_decrypt() because other code
should not need to guess whether zero-length input is
allowed or not.

Patch attached.

> Also, what other pgcrypto routines might have similar bugs?

Well, PGP code accesses anything thru wrappers, so should be OK.
Rest of the code does not try to parse user data, just passes
it thru.

Except armor()/dearmor(), which does lot of pointer-juggling.
I can do a review of that, just in case.

--
marko

"Marko Kreen" <markokr@gmail.com> writes:
> The fix should be in combo_decrypt() because other code
> should not need to guess whether zero-length input is
> allowed or not.

> Patch attached.

Thanks -- applied in all branches back to 7.3.

            regards, tom lane