[Mesa-dev] [PATCH 12/15] egl: add eglCreateImage

[Marek Olšák]

I don't understand. Using size_t should prevent the integer overflow.
Is there anything else wrong other than no fail path for malloc? I
also don't understand how calloc can help here.

Marek


On Wed, May 27, 2015 at 9:07 PM, Chad Versace <chad.versace at intel.com> wrote:
> On Fri 15 May 2015, Emil Velikov wrote:
>> On 12/05/15 22:54, Marek Olšák wrote:
>> > From: Marek Olšák <marek.olsak at amd.com>
>> >
>> > ---
>> >  src/egl/main/eglapi.c | 38 ++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 38 insertions(+)
>> >
>> > diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
>> > index 6457798..34a113b 100644
>> > --- a/src/egl/main/eglapi.c
>> > +++ b/src/egl/main/eglapi.c
>> > @@ -251,6 +251,30 @@ _eglUnlockDisplay(_EGLDisplay *dpy)
>> >  }
>> >
>> >
>> > +static EGLint *
>> > +_eglConvertAttribsToInt(const EGLAttrib *attr_list)
>> > +{
>> > +   EGLint *int_attribs = NULL;
>> > +
>> > +   /* Convert attributes from EGLAttrib[] to EGLint[] */
>> > +   if (attr_list) {
>> > +      int i, size = 0;
>> > +
>> > +      while (attr_list[size] != EGL_NONE)
>> > +         size += 2;
>> > +
>> > +      if (size) {
>> > +         size += 1; /* add space for EGL_NONE */
>> > +         int_attribs = malloc(size * sizeof(int_attribs[0]));
>> > +
>> > +         for (i = 0; i < size; i++)
>> > +            int_attribs[i] = attr_list[i];
>
>> In the unlikely event that malloc fails, it'll be nice to not crash.
>
> NAK.
>
> There is a stack overflow vulnerability here, even when malloc succeeds.
> An attacker can pass a very large but valid `EGLint *attrib_list` into
> an EGL entry point, forcing the size calculation given to malloc to
> overflow to a small positive integer.  Then _eglConvertAttribsToInt will
> blithely copy a portion (perhaps most) of the attacker's attrib list onto
> the stack!
>
> To prevent the stack overflow, _eglConvertAttribsToInt should use
> calloc() and abort if allocation fails.