[erlang-patches] [PATCH] ei: integer overflow in string/atom encoding

[Mikael Pettersson]

Michael Santos writes:
 > ei_encode_atom() and ei_encode_string() use strlen() to get the length
 > of the buffer. As strlen() returns an unsigned long long and both ei

size_t is unsigned but may or may not be long long

 > functions take a signed integer, the length fields may overflow.

Indeed.

 > 
 > Note: the index may still overflow.
 > 
 > Check the results of strlen can be held in a signed integer.

The change feels like a kludge.  IMO ei_encode_{atom,string}_len() should be
fixed to take size_t lengths rather than signed int lengths.  Is there any
reason why that wouldn't work?

/Mikael

p.s. Thank you for inlining the patch and providing a rationale, most git
users on this list don't do that which makes peer review difficult.

 > ---
 >  lib/erl_interface/src/encode/encode_atom.c   |    4 ++++
 >  lib/erl_interface/src/encode/encode_string.c |    6 +++++-
 >  2 files changed, 9 insertions(+), 1 deletions(-)
 > 
 > diff --git a/lib/erl_interface/src/encode/encode_atom.c b/lib/erl_interface/src/encode/encode_atom.c
 > index 69f2d14..0efa204 100644
 > --- a/lib/erl_interface/src/encode/encode_atom.c
 > +++ b/lib/erl_interface/src/encode/encode_atom.c
 > @@ -17,12 +17,16 @@
 >   * %CopyrightEnd%
 >   */
 >  #include <string.h>
 > +#include <limits.h>
 >  #include "eidef.h"
 >  #include "eiext.h"
 >  #include "putget.h"
 >  
 >  int ei_encode_atom(char *buf, int *index, const char *p)
 >  {
 > +    size_t len = strlen(p);
 > +
 > +    if (len >= INT_MAX) return -1;
 >      return ei_encode_atom_len(buf, index, p, strlen(p));
 >  }
 >  
 > diff --git a/lib/erl_interface/src/encode/encode_string.c b/lib/erl_interface/src/encode/encode_string.c
 > index 1d342cb..593bbf2 100644
 > --- a/lib/erl_interface/src/encode/encode_string.c
 > +++ b/lib/erl_interface/src/encode/encode_string.c
 > @@ -17,6 +17,7 @@
 >   * %CopyrightEnd%
 >   */
 >  #include <string.h>
 > +#include <limits.h>
 >  #include "eidef.h"
 >  #include "eiext.h"
 >  #include "putget.h"
 > @@ -24,7 +25,10 @@
 >  
 >  int ei_encode_string(char *buf, int *index, const char *p)
 >  {
 > -    return ei_encode_string_len(buf, index, p, strlen(p));
 > +    size_t len = strlen(p);
 > +
 > +    if (len >= INT_MAX) return -1;
 > +    return ei_encode_string_len(buf, index, p, len);
 >  }
 >  
 >  int ei_encode_string_len(char *buf, int *index, const char *p, int len)
 > -- 
 > 1.7.0.4
 > 
 > _______________________________________________
 > erlang-patches mailing list
 > erlang-patches@REDACTED
 > http://erlang.org/mailman/listinfo/erlang-patches
 >