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.vers...@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.ol...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev