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

[Ian Romanick]

On 05/23/2013 08:44 AM, Alan Coopersmith wrote:
> busIdStringLength is a CARD32 and needs to be bounds checked before adding
> one to it to come up with the total size to allocate, to avoid integer
> overflow leading to underallocation and writing data from the network past
> the end of the allocated buffer.
>
> Reported-by: Ilja Van Sprundel <ivansprundel at ioactive.com>
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
>   src/glx/XF86dri.c |    7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/glx/XF86dri.c b/src/glx/XF86dri.c
> index b1cdc9b..8f53bd7 100644
> --- a/src/glx/XF86dri.c
> +++ b/src/glx/XF86dri.c
> @@ -43,6 +43,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>   #include <X11/extensions/Xext.h>
>   #include <X11/extensions/extutil.h>
>   #include "xf86dristr.h"
> +#include <limits.h>
>
>   static XExtensionInfo _xf86dri_info_data;
>   static XExtensionInfo *xf86dri_info = &_xf86dri_info_data;
> @@ -201,7 +202,11 @@ XF86DRIOpenConnection(Display * dpy, int screen, drm_handle_t * hSAREA,
>      }
>
>      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?

Even this is only a problem because of calloc's zero size handling behavior:

     If nmemb or size is 0, then calloc() returns either NULL, or
     a unique pointer value that can later be successfully passed
     to free().

Good times.

> +      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.

>            UnlockDisplay(dpy);
>            SyncHandle();
>