Thread: numeric access out of bounds

Happened to notice this:

postgres=# select numeric_send('NaN');   numeric_send    
--------------------\x00007f7ec0000000
(1 row)

7f7e obviously screams "accessing memory beyond the end of data", and
indeed this is so: init_var_from_num, when passed a NaN, accesses two
bytes after the input. This probably goes unnoticed because a NaN is 6
bytes including varlena header, so the next two bytes wouldn't cause a
segfault (and clients shouldn't care about the value since the NaN flag
is set), but it's still clearly wrong.

I can see two possible fixes: one to correct the assumptions in the
macros, the other to check for NaN before calling init_var_from_num in
numeric_send (all the other functions seem to do this check explicitly).
Which would be preferable?

-- 
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> I can see two possible fixes: one to correct the assumptions in the
> macros, the other to check for NaN before calling init_var_from_num in
> numeric_send (all the other functions seem to do this check explicitly).
> Which would be preferable?

I'm inclined to think special-casing NaN in numeric_send is the thing to
do, since it is that way everywhere else.  If we could push down all the
special casing into init_var_from_num then that would probably be better,
but in most cases that looks unattractive.

Perhaps it'd make sense to add an Assert that the input isn't NaN in
init_var_from_num?  That wouldn't really do all that much to forestall
future similar errors, since it wouldn't expose an oversight unless the
code was actually tested with NaN input ... but it's better than nothing.

Looks like set_var_from_num has same issue, too.
        regards, tom lane

I wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>> I can see two possible fixes: one to correct the assumptions in the
>> macros, the other to check for NaN before calling init_var_from_num in
>> numeric_send (all the other functions seem to do this check explicitly).
>> Which would be preferable?

> I'm inclined to think special-casing NaN in numeric_send is the thing to
> do, since it is that way everywhere else.  If we could push down all the
> special casing into init_var_from_num then that would probably be better,
> but in most cases that looks unattractive.

After taking a closer look I realized that you were right with your first
alternative: the field access macros are just plain wrong, and we should
fix 'em.  Done now.
        regards, tom lane