[Mesa-dev] [PATCH:mesa 1/2] integer overflow in XF86DRIOpenConnection() [CVE-2013-1993 1/2]

[Alan Coopersmith]

On 05/23/13 11:07 AM, Ian Romanick wrote:
> On 05/23/2013 08:44 AM, Alan Coopersmith wrote:
>>      if (rep.length) {
>> -      if (!(*busIdString = calloc(rep.busIdStringLength + 1, 1))) {
>> +      if (rep.busIdStringLength < INT_MAX)
>> +         *busIdString = calloc(rep.busIdStringLength + 1, 1);
>
> But calloc takes size_t, and size_t is unsigned.  That makes this look a little
> weird.  The problem is when rep.busIdStringLength is INT_MAX, the problem occurs
> when it's UINT_MAX.  Right?

Right - UINT_MAX would cause overflow, but we used INT_MAX in most of the checks
for these in the X libraries to avoid issues if other parts of the code try to
treat it as signed, and really, once your string length hits 2gb you're in
ludicrous territory anyway, and best not to waste time trying to map all those
pages.

>> +      else
>> +         *busIdString = NULL;
>> +      if (*busIdString == NULL) {
>>            _XEatData(dpy, ((rep.busIdStringLength + 3) & ~3));
>
> Doesn't this have a similar overflow issue?  If rep.busIdStringLength is
> UINT_MAX-2, the result is 0.

Yes - in that case though you'll just not throw away enough data, and on
modern systems, trigger an xcb assertion that there's unread data left in
the response.   In the X libraries I added a new _XEatDataWords API that
takes the packet length from req.length and applies overflow checks in one
place instead of every caller, but I didn't take the time to figure out
how to add new autoconf checks to Mesa to do that here.

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc