I reviewed glamor-egl version 0.5.1-0ubuntu1 as checked into saucy. This
should not be considered a full security audit, but rather a quick gauge
of code quality.
- glamor uses OpenGL primitives to provide corresponding accelerated 2d
functionality without necessarily requiring hardware support for
accelerated 2d, nor without requiring specific drivers to use any
hardware support for 2d acceleration.
- Build-depends on xutils, x11proto-dri2, libdrm, xserver-xorg,
libgl1-mesa, libpixman-1, libegl1-mesa, libgbm
- Runtime-depends on libEGL, libgbm, libGL
- Does not use cryptography
- Does not daemonize
- Does no networking
- Runs with elevated privileges, though does not manage privileges itself
- No initscripts
- No dbus
- No setuid
- No binaries in bin/
- No sudo fragments
- No cronjobs
- No test suite
- Clean build logs
- No subprocesses spawned
- Some unclean memory management, these two may even be vulnerabilities:
- _glamor_poly_lines() unchecked malloc() return rects; n == 1 called from
outside the library might be a problem
- _pixman_region_init_clipped_rectangles() num_rects passed unchecked to
malloc(), appears callable from outside the codebase with arbitrary
arguments, might be a problem
- Several unchecked malloc() return values:
- 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
- Some code is not sufficiently defensive:
- glamor_create_composite_fs() unguarded divide-by-zero possibility in
relocate_texture (wh.x, wh.y; wh.xy divide is handled via RepeatNone)
- glamor_create_composite_fs() unguarded divide-by-zero possibility in
rel_sampler (wh.xy)
I followed these call chains far enough to wonder if the necessary
constraints could always be enforced.
- Logging looked safe
- Environment variable only used for debugging, looked safe
- Does not manage privileges
- Does not use cryptography
- Does not use networking
- Does not use tmp files
- Does not use webkit
- Does not use javascript
- glamor_pixmap_attach_fbo() switch case fall-through without annotation,
probably safe but lack of annotation is annoying.
Packaging needs attention:
- Extraneous files debian/changelog.{REMOTE,BASE,LOCAL,BACKUP}.26867
- Patches without DEP-3 patch tags
- Files changed outside of patch system
The majority of the code is well-written and moderately defensive. The few
potential issues I've raised here are surprising blemishes given the
quality seen elsewhere in the library.
I'll ask upstream about the potential issues.
Before this can be promoted to main someone needs to fix the packaging
issues: extra changelog files, files changed outside the patch system,
DEP-3 patch tagging.
Security team ACK for promoting to main, conditional upon fixed
packaging.
Thanks
** Changed in: glamor-egl (Ubuntu)
Assignee: Seth Arnold (seth-arnold) => (unassigned)
--
You received this bug notification because you are a member of Ubuntu-X,
which is subscribed to glamor-egl in Ubuntu.
https://bugs.launchpad.net/bugs/1227919
Title:
[MIR] glamor-egl
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/glamor-egl/+bug/1227919/+subscriptions
_______________________________________________
Mailing list: https://launchpad.net/~ubuntu-x-swat
Post to : ubuntu-x-swat@lists.launchpad.net
Unsubscribe : https://launchpad.net/~ubuntu-x-swat
More help : https://help.launchpad.net/ListHelp
