[Bug 1208800] Re: [MIR] click

[Jamie Strandboge]

I performed a cursory code review. Click is coded well and defensively and I am not concerned about its maintenance. There are a couple of interesting things to note:
 * uses LD_PRELOAD to load /usr/lib/x86_64-linux-gnu/click/libclickpreload.so to override various libc functions similar to fakeroot
 * the model for dropping privileges is that click runs as root and drops for certain operations, such as when calling dpkg or running user hooks. This works fine but it should be noted that some operations such as opening the arfile and examining it or loading the manifest file happen as root. Because this is python, a crafted click package would have to most likely exploit a python bug, but a useful hardening measure might be to perform input verification on these files as non-root
 * _drop_privileges() is implemented slightly differently in three different places in the code, but coded correctly in each
 * hooks.py _run_commands() uses shell=True. hooks.py reads in the hooks file in /usr/share/click/hooks using debian.deb822 and self["exec"] is set and executed unconditionally based on the contents of the hooks file. This is fine as implemented because you need privilege to modify files in /usr/share/click/hooks but it should be noted in case non-system hooks are ever considered. Also, because of the click privilege model, if click could be subverted to perform a file write while running privileged, this provides a convenient way to turn that directly to code execution. A useful hardening measure might be to perform input sanitization for "Exec" in the manifest file.

No blockers. ACK from the security team.

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to click in Ubuntu.
https://bugs.launchpad.net/bugs/1208800

Title:
  [MIR] click

Status in “click” package in Ubuntu:
  Fix Committed

Bug description:
  Availability: In universe for all architectures.

  Rationale: Click is the lightweight application packaging system for
  Ubuntu Touch:
  https://blueprints.launchpad.net/ubuntu/+spec/foundations-1305-click-
  package

  Security: Ubuntu-native and a new development, so no security history.
  The click program and the PackageKit plugin are security-sensitive in
  that it typically starts as root and it installs untrusted
  applications (which then run under AppArmor confinement).  The design
  has been discussed extensively with the security team, although I
  don't know if they've done a full code review.

  QA: Requires no configuration.  No important outstanding bugs.
  Maintained by Canonical (the Ubuntu Foundations team).  Includes a
  test suite which is run at build time.

  Dependencies: All in main.

  Standards compliance: As far as I know this complies with Debian
  Policy 3.9.4.  Applications installed by click are currently unpacked
  into /opt/click.ubuntu.com/, although this may change in at least some
  cases; the click package creates this directory in its postinst (which
  is skating around the edges of policy a bit, but I think it's
  reasonable).

  Maintenance: Ubuntu Foundations team; foundations-bugs and I are set
  as bug contacts.

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