Marek Olšák <mar...@gmail.com> writes:
> 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
"size * sizeof(int_attribs[0])" may overflow and thus wrap to a small
number. Using calloc, you'd have "calloc(size, sizeof(int_attribs[0]))",
moving the overflow inside calloc(). So if calloc() does its job
properly, it will protect against it.
eirik
> 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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
