Hi,

It is been observed on RANDOMIZE_ALLOCATED_MEMORY enabled PG95 build that
chkpass is failing because of uninitialized memory and seems showing false
alarm. I have tried to add code snippets to explain as following i.e.

postgres=# CREATE EXTENSION chkpass;
> WARNING:  type input function chkpass_in should not be volatile
> CREATE EXTENSION
> postgres=# CREATE TABLE test_tbl ( name text, pass chkpass );
> CREATE TABLE
> postgres=# INSERT INTO test_tbl VALUES('foo','foo1');
> WARNING:  type chkpass has unstable input conversion for "foo1"
> LINE 1: INSERT INTO test_tbl VALUES('foo','foo1');
>                                           ^
> INSERT 0 1


It is giving warning "type chkpass has unstable input conversion" because
chkpass structure hold uninitialized memory area’s that are left unused.
When chkpass_in() is called with input “foo1”, it allocate 16 bytes of
randomized memory for chkpass i.e.

contrib/chkpass/chkpass.c

> /*
>  * This is the internal storage format for CHKPASSs.
>  * 15 is all I need but add a little buffer
>  */
> typedef struct chkpass
> {
>     char        password[16];
> } chkpass;


chkpass_in()

>     result = (chkpass *) palloc(sizeof(chkpass));


*result memory*

> 0x7ffc93047a68: 0xdd 0xde 0xdf 0xe0 0xe1 0xe2 0xe3 0xe4
> 0x7ffc93047a70: 0xe5 0xe6 0xe7 0xe8 0xe9 0xea 0xeb 0xec
>

It copies string (16 bytes max) returned from crypt() function, it copies
until null character reached i.e.

    strlcpy(result->password, crypt_output, sizeof(result->password));


*crypt_output memory*

> 0x7fff7d1577f0: 0x6a 0x6e 0x6d 0x54 0x44 0x53 0x55 0x33
> 0x7fff7d1577f8: 0x4b 0x48 0x52 0x55 0x6f 0x00 0x00 0x00


*result memory*

> 0x7ffc93047a68: 0x6a 0x6e 0x6d 0x54 0x44 0x53 0x55 0x33
> 0x7ffc93047a70: 0x4b 0x48 0x52 0x55 0x6f 0x00 0xeb 0xec


It left remaining last 2 byte left untouched. It is the cause for "unstable
input conversion” warning.

I think these uninitialized memory areas are false alarms. Along with this
there seems another issue that makes chkpass troubling
RANDOMIZE_ALLOCATED_MEMORY comparison logic is that crypt() function is
being passed with random salt value for DES based result. PFA patch, to
generate consistent results, it uses constant salt value.

It seems a minor inconvenience but it will make results better. Please do
let me know if I missed something or more information is required. Thanks.

Regards,
Muhammad Asif Naeem

Attachment: chkpass_RANDOMIZE_ALLOCATED_MEMORY_v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to