On 08/15/2014 11:55 AM, Marko Tiikkaja wrote:
Hi,

On 8/8/14 3:18 PM, I wrote:
Currently there's no way to generate or extract armor headers from the
PGP armored format in pgcrypto.  I've written a patch to add the
support.

Latest version of the patch here, having fixed some small coding issues.

This coding style gives me the willies:

        guess_len = pgp_armor_enc_len(data_len, num_headers, keys, values);
        res = palloc(VARHDRSZ + guess_len);

        res_len = pgp_armor_encode((uint8 *) VARDATA(data), data_len,
                                                           (uint8 *) 
VARDATA(res),
                                                           num_headers, keys, 
values);
        if (res_len > guess_len)
                ereport(ERROR,
                                
(errcode(ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION),
                                 errmsg("Overflow - encode estimate too 
small")));

That was OK before this patch, as the length calculation was simple enough to verify (although if I were writing it from scratch, I would've written it differently). But with this patch, it gets a lot more complicated, and I can't easily convince myself that it's correct.

pgp_armor_enc_len might be vulnerable to integer overflow. Consider 1GB worth of keys, 1GB worth of values, and 1GB worth of data. I'm not sure if you can quite make it overflow a 32-bit unsigned integer, but at least you can get nervously close, e.g if you use use max-sized key/value arrays, with a single byte in each key and value. Oh, and if you use a single-byte server encoding and characters that get expanded to multiple bytes in UTF-8, you can go higher.

So I think this (and the corresponding dearmor code too) should be refactored to use a StringInfo that gets enlarged as needed, instead of hoping to guess the size correctly beforehand. To ease review, might make sense to do that as a separate patch over current sources, and the main patch on top of that.

BTW, I'm surprised that there is no function to get all the armor headers. You can only probe for a particular one with pgp_armor_headder, but there is no way to list them all, if you don't know what you're looking for.

- Heikki



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