glamor-egl, parameter checking

[Zhigang Gong]

Thanks for reporting these issues. I will investigate them after I back from
holiday, say after 8/10.

-----Original Message-----
From: xorg-devel-bounces+zhigang.gong=linux.intel.com at lists.x.org
[mailto:xorg-devel-bounces+zhigang.gong=linux.intel.com at lists.x.org] On
Behalf Of Seth Arnold
Sent: Thursday, September 26, 2013 9:29 AM
To: xorg-devel at lists.x.org
Cc: security at ubuntu.com
Subject: glamor-egl, parameter checking

Hello, I recently gave glamor-egl a very quick audit, and found some issues
that I wanted to run past someone who would know the code better than I do.

- _glamor_poly_lines():
  - unchecked malloc() return value
  - if n = 1 is passed to the function, the malloc might allocate zero
    bytes:
            rects = malloc(sizeof(xRectangle) * (n - 1));
  Can n = 1 be a realistic input to this function? It appears to be
  callable from outside the library with arbitrary inputs.
  Is this safe?

- _pixman_region_init_clipped_rectangles()
  - The unsigned int num_rects argument is passed, unchecked, to
    boxes = malloc(sizeof(pixman_box16_t) * num_rects);
    -- can a large-enough value of num_rects cause a multiplication
    overflow here, allocating less memory than necessary?
  It appears to be callable from outside the library with arbitrary
  inputs. Is this safe?

- glamor_create_composite_fs() appears to have two unguarded divisions
  in relocate_texture that might result in divide-by-zero, wh.x and wh.y
  -- are these guarded somewhere else?

- glamor_create_composite_fs() appears to have an unguarded division in
  rel_sampler that might result in divide-by-zero, wh.xy -- is this
  guarded somewhere else?

- glamor_pixmap_attach_fbo() has a switch statement that uses
  fall-through after a block of code, but there's no comment nearby to
  assure the reader that it is intentional. Is it intentional? :)

There are several unchecked memory allocations:
  - glamor_compile_glsl_prog() unchecked malloc() return value
  - glamor_egl_init() unchecked calloc() return value glamor_egl
  - glamor_compute_clipped_regions_ext() unchecked calloc() return value
    result_regions
  - __glamor_compute_clipped_regions() unchecked calloc() return value
    clipped_regions
  - glamor_composite_largepixmap_region() unchecked malloc() return value
    source_pixmap_priv

(While it's true malloc() will almost never return NULL, someone may someday
wish to run this code with overcommit turned off, and it'd be better to be
safe.)

Thanks