touch-packages team mailing list archive

[Rolf Leggewie <1223586@xxxxxxxxxxxxxxxxxx>]

saucy has seen the end of its life and is no longer receiving any
updates. Marking the saucy task for this ticket as "Won't Fix".

** Changed in: libhybris (Ubuntu Saucy)
       Status: Confirmed => Won't Fix

-- 
You received this bug notification because you are a member of Ubuntu
Touch seeded packages, which is subscribed to libhybris in Ubuntu.
https://bugs.launchpad.net/bugs/1223586

Title:
  security team audit of libhybris

Status in libhybris package in Ubuntu:
  Confirmed
Status in libhybris source package in Saucy:
  Won't Fix

Bug description:
  Since https://bugs.launchpad.net/ubuntu/+source/libhybris/+bug/1188213 is
  already labeled "fix-committed", this is a new bug to address issues
  raised during the MIR audit.

  Here's the comment I filed on the other bug report:

  I reviewed libhybris version 0.1.0+git20130606+c5d897a-0ubuntu21 from
  saucy. This should not be considered a complete security audit, but
  instead a quick gauge of code cleanliness.

  - The package provides a shim between Android services and glibc standard
    C library, it hooks a large number of system service functions to
    provide the subtly different semantics expected by Android applications.
  - Build-depends upon autotools, gcc, g++, quilt, pkg-config, libgles2-mesa-dev
  - Does not itself use cryptography
  - Limited use of networking, for the debugger shim
  - Does not itself daemonize
  - Can run with elevated privileges
  - No initscripts
  - No dbus services
  - No setuid/setgid executables
  - Provides several binaries:
    ./usr/bin/test_recorder
    ./usr/bin/test_ui
    ./usr/bin/test_camera
    ./usr/bin/test_egl
    ./usr/bin/test_media
    ./usr/bin/test_sf
    ./usr/bin/test_audio
    ./usr/bin/test_input
    ./usr/bin/test_glesv2
    ./usr/bin/test_sensors
    ./usr/bin/test_gps
    ./usr/bin/test_lights
    ./usr/bin/setprop
    ./usr/bin/getprop
  - No sudo fragments
  - No cron jobs
  - Build logs are troubling:
    - many instances of pointer / integer size mismatch warnings which mean
      this code will require effort to port to a 64 bit environment.
    - hybris/libsync/sync.c calls free(3) without #include <stdlib.h>
      and thus uses the wrong prototype. (C assumes functions without
      prototypes are passing integer arguments, not a pointer.)
    - hybris/egl/egl.c eglGetProcAddress uses ugly return type hack,
      warning message "return from incompatible pointer type" looks
      like the ugly hack hasn't been completely used correctly
    - hybris/common/hooks.c, pthread_attr_getstackaddr and
      pthread_attr_setstackaddr are deprecated, should be replaced with
      pthread_attr_getstack and pthread_attr_setstack.

  Severe-looking problem:

  - hybris/common/ics/linker.c nothing sets program_is_setuid variable
    This means the environment scrubbing is not performed, fds 0, 1, 2
    aren't opened to /dev/null if they were closed, and LD_LIBRARY_PATH and
    LD_PRELOAD get to freely modify the address space of the setuid
    executable.

  Someone needs to investigate if these are problems:

  - hybris/common/*/linker.c The fds 0, 1, 2 are only nullified for programs
    with program_is_setuid set. Since programs tend to assume these fds are
    always open at exec(), the linker should probably set them open
    regardless of program_is_setuid value.
  - hybris/egl/egl.c _init_androidegl() uses environment variables for
    loading LIBEGL and LIBGLESV2, no safety checks in place
  - hybris/egl/ws.c _init_ws() uses environment variable EGL_PLATFORM for
    loading elgplatform_%s.so, no safety checks in place
  - hybris/common/hooks_shm.c _hybris_shm_init() uses 0660, is this
    correct? Why not 0600? This code performs no ownership checks, another
    user could create the shm segment, mode 0666, and trouble ensues.
  - hybris/common/hooks_shm.c _hybris_shm_fd is left opened beyond needed
    lifetime

  The 64-bit-unclean code is not an issue at the moment, though this design
  decision may represent a significant technical debt that must be paid in
  the future. Ideally, we would not need this library by the time 64 bit
  targets are available.

  
  libhybris as it stands should not be in main, though it is close. The
  conditions for including libhybris:

  The ics/linker.c needs program_is_setuid to be set properly.

  All the linkers need to nullify fds on all executables, not just
  setuid/setgid/etc executables, unless we can demonstrate there are actual
  problems with this approach.

  Someone more familiar with the code needs to explain the ramifications
  of unchecked library loading via the LIBEGL, LIBGLESV2, EGL_PLATFORM
  environment variables. These variables might need to be specially handled
  in the linker for safety.

  Someone more familiar with the code needs to inspect the shared memory
  segment handling and either explain why the relaxed 0660 mode is fine
  -- and why there are no owner checks for the already-created case --
  or address these issues.

  Someone more familiar with the code needs to inspect the _hybris_shm_fd
  file descriptor and either explain why the fd cannot be closed (and use
  _hybris_shm_data variable instead) or explain why it is not an issue,
  or address this issue.

  Someone more familiar with the code needs to inspect the libsync/sync.c,
  egl/egl.c, and common/hooks.c warning messages and either correct the
  warnings or explain why they cannot be corrected.

  Please address the above issues before we ship this package.

  
  Thank you

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/libhybris/+bug/1223586/+subscriptions