[erlang-bugs] ei_decode_list_header and big lists

[Michael Santos]

On Fri, May 20, 2011 at 10:44:34AM +0200, Björn-Egil Dahlberg wrote:
> On 2011-05-20 02:59, Olivier Girondel wrote:
> >What if...
> >
> >I send a big list (eg, 80000 elements) to a C-port ?
> >
> >According to
> >http://www.erlang.org/doc/man/ei.html#ei_decode_list_header
> >
> >the list length will be stored into an "int",
> >which obviously will lead to strange behavior ?
> 
> Well, the length should probably be stored as an Uint32 in ei but it
> isn't, clearly a miss. An int on most C compilers will be a signed
> 32-bit integer, so all the bits and pieces will be there. But, with
> the wrong type ...
> 
> Reference, http://www.erlang.org/doc/apps/erts/erl_ext_dist.html

The index is an int and so can overflow as well.

There are a few other changes that would make ei safer to use:

1. ei_encode_string and ei_encode_atom use strlen() to get the length
of the buffer. It looks as if a size_t is an unsigned long long which
can overflow the length in ei_encode_string_len.

I guess the appropriate thing to do here is to check if strlen(buf) >=
INT_MAX.

2. There doesn't seem to be a safe way to call ei_get_type() on some
types. For example, the encoded version of the binary <<"foo">> is:

<<131,109,0,0,0,3,102,111,111>>

If I send fake data like:

<<131,109,255,255,255,255,102,111,111>>

The size parameter (a signed int) in ei_get_type() will overflow.

The ei doc has a nice warning about having an appropriate buffer size.
Might be nice to have an example of using ei safely as well. Something
like using ei_get_type() to get the message length and failing if it
exceeds the caller's expectations:

    n = read(socket, buf, 1024);
    ei_get_type(buf, &index, &type, &size);
    if (size > n)
        errx(EXIT_FAILURE, "ei length > buf size!");

This still isn't quite right though because it doesn't include the size
of the message header. So maybe a new function like ei_get_message_len()
is needed.

Not sure if anyone is using ei with untrusted data, so these aren't a
huge concern.