[MIR] htop

Bug #1644364 reported by Joshua Powers on 2016-11-23
16
This bug affects 1 person
Affects Status Importance Assigned to Milestone
htop
Fix Released
Unknown
htop (Debian)
Fix Released
Unknown
htop (Ubuntu)
Medium
Unassigned

Bug Description

[Availability]
The package is currently in universe with no deltas with Debian.

[Rationale]
The Ubuntu Server team wishes to seed the package in ubuntu-server image.

The package provides an interactive process viewer that is more complex and feature rich than top, while still being a text-mode application. The package is useful for system administrators all the way to general users to get a better idea of what is going on with their system.

[1] https://hisham.hm/htop/index.php

[Security]
There has been a single CVE [1] for htop back in 2008 around version 0.7 (now on 2.0.2). The source package comes with one binary package, which contains a single binary under /usr/bin.

[2] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5076

[Quality assurance]
There is no configuration required and no debconf questions asked by the package. The package currently is built and ran across all release architectures.

The package does have a debian/watch file [3].

There are a 14 bugs in the Ubuntu bug tracker [4] and 26 in the Debian bug tacker [5]. None of which are related to data corruption, system crashing, or other catastrophic issues.

[3] https://anonscm.debian.org/cgit/collab-maint/htop.git/tree/debian/watch
[4] https://bugs.launchpad.net/ubuntu/+source/htop
[5] https://bugs.debian.org/cgi-bin/pkgreport.cgi?repeatmerged=no&src=htop

[Dependencies]
All three run time dependencies [6] are in main: libc6, libtinfo5, and libncursesw5

All build dependencies [7] are in main: debhelper, dh-autoreconf, dpkg-dev, libncurses5-dev, libncursesw5-dev, python-minimal

I am ignoring libhwloc-dev and libkvm-dev as build dependencies as libhwloc-dev is marked as 'not linux-any' and libkvm-dev is marked as 'kfreebsd-any'.

[6] http://packages.ubuntu.com/yakkety/htop
[7] http://packages.ubuntu.com/source/yakkety/htop

[Standards compliance]
The package has no major lintian [8] problems.

[8] https://<email address hidden>#htop

[Maintenance]
We can continue to keep this package synced directly with Debian. It is not a volatile package with many changes coming.

[Background information]
The general purpose and context of the package is explained in the package's debian/control file. This package has not had a different name in the past.

Michael Terry (mterry) wrote :

Your rationale section doesn't explain what you want to do with htop. Is the Ubuntu Server team looking to seed htop on the ubuntu-server image?

Changed in htop (Ubuntu):
status: New → Incomplete
Joshua Powers (powersj) wrote :

Yes - I have updated the section.

description: updated
Changed in htop (Ubuntu):
status: Incomplete → New
Michael Terry (mterry) on 2016-11-30
Changed in htop (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (cyphermox)

htop needs a team subscriber, and a review/cleanup of old bugs in Launchpad. I see no other issues with the package; but please fix the above. Thanks!

Changed in htop (Ubuntu):
status: New → Incomplete
Changed in htop (Ubuntu):
milestone: none → ubuntu-17.01
Joshua Powers (powersj) wrote :

The server team is now a team subscriber.

The bug backlog was evaluated and replied to each. Of the two actionable items, one was found only in precise and one was found only in trusty, however I am not bothering with an SRU for either due to lack of demand, precise falling off support shortly, and lack of where the fix is for the trusty defect.

Changed in htop (Ubuntu):
status: Incomplete → New

This is a complex package and it has already had a CVE, and a similar bug has been recently opened that looks as though packages being "hidden" might still be an issue:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=793106

I see no particular issues with the package (other than the complexity of its source), but I think it would benefit a quick Security review.

Changed in htop (Ubuntu):
assignee: Mathieu Trudel-Lapierre (cyphermox) → Ubuntu Security Team (ubuntu-security)
Changed in htop (Ubuntu):
milestone: ubuntu-17.01 → ubuntu-17.10
Emily Ratliff (emilyr) wrote :

Upstream uses the throw a tarball over the wall model of development rather than open development in a tracked repo. This greatly complicates security support for this type of package.

Emily Ratliff (emilyr) wrote :

Scratch that last comment, the repo is at https://github.com/hishamhm/htop

Seth Arnold (seth-arnold) wrote :

I reviewed htop version 2.0.2-1 as checked into artful. This should not be
considered a full security audit but rather a quick gauge of
maintainability.

- One CVE in Ubuntu's CVE database, for printing unsanitized data to the
  screen. (Specifically process names; I expect there's more of this.)

- htop is a prettier top
- Build-Depends: debhelper, dh-autoreconf, dpkg-dev, libncurses5-dev,
  libncursesw5-dev, python-minimal
- No cryptography
- No networking
- Does not daemonize
- No pre/post inst/rm scripts
- No initscripts
- No setuid executables
- htop executable in PATH
- No sudo fragments
- No udev rules
- No test suite at build
- Didn't see cronjobs
- Noisy build logs, including missing seteuid() return handling

- htop can spawn strace; the execlp() itself looked fine, but the
  seteuid() nearby does no error checking at all
- Some memory-management wrappers are used; there's a RichString
  abstraction that tries to be friendly with memory management, but
  I'm afraid that not all functions properly handle the distinctions
  between pointer pointing into the struct, and pointer pointing at
  malloc()ed space.
- Most filenames are constructed via CPP token-pasting
- Almost no logging
- Uses LC_CTYPE LC_ALL HTOPRC HOME XDG_CONFIG_HOME environment
  variables; I didn't inspect these closely
- Some privileged operations are used, sometimes without checking errors
  returns.
- No cryptography
- No networking
- The bulk of the code may be executing privileged; transitions to
  unprivileged may not be safe
- No temporary files
- No webkit
- No policykit
- No javascript
- Clean cppcheck

Most of the code looked alright and it might be suitable for use on
personal desktops however I don't think htop is sufficiently paranoid
to be run as root on systems with untrusted unprivileged users. I don't
believe that the benefits of htop outweigh the risks at this time,
so security team NAK for promoting htop to main.

Feel free to re-apply after adding error handling to seteuid() and
snprintf() calls and converting the sprintf() calls with floats to
snprintf() calls; and investigate what happens if a 200-byte RichString
is extended by another 200 bytes. (My guess is it'll just buffer-overflow
and scribble on unrelated memory.)

Here's some notes I took while reviewing htop, I hope they're useful:

- NOT OKAY (void) seteuid(getuid());

- The code makes many assumptions that floats are "safe" when printing
  them. Floats can overflow buffers in unexpected ways. snprintf()
  should be used almost anywhere that floats are being printed.

- The snprintf() error return should be checked everywhere.

- UptimeMeter_updateValues() assumes uptimes are less than 9999 days
  without any error handling.

- RichString_extendLen() looks like it's missing support for extending a
  rich string from e.g. 200 bytes by another 200 bytes.

- LinuxProcessList_readStatFile() could very easily be tricked into buffer
  overflows if /proc/pid/stat files are maliciously constructed (say via
  container filesystems, private filesystem namespaces, etc)

Thanks

Changed in htop (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Joshua Powers (powersj) wrote :

I am withdrawing this MIR request and marking this invalid.

While a very useful application, based on the security review it seems
this is not a good candidate at the current time. There are more than a
few small changes required to get this accepted. This review will be
passed onto the maintainer, but not acted on by myself.

htop appears to be a good opportunity to be delivered as a snap with
confinement to get prevent concerns from the security review. Once
there is a defined process for a snap-based MIR I can reapply at that
time.

Thanks to folks for their time and reviews.

Changed in htop (Ubuntu):
status: New → Invalid
Seth Arnold (seth-arnold) wrote :

Josh, I had a very nice mail from the author; he intends to address most of my issues with the next release. I believe it would be worthwhile addition after he does so -- it is a very popular request from users and it's undeniably pretty. :)

Thanks

Seth Arnold (seth-arnold) wrote :

I reviewed the fixes Hisham has committed to the htop git tree and liked what I saw.

Security team ACK for promoting htop > 2.0.2 to main. (The fixes I've asked for aren't yet in a release.)

Thanks

Joshua Powers (powersj) wrote :

Thanks Seth! Marking bug new again.

Changed in htop (Ubuntu):
status: Invalid → New
importance: Undecided → Medium

This is still in a locked state - not sure if that will be in 18.04.
The ack is for >2.0.2

The changes are in git for a while, but no version with them was released yet.
Next steps are:
1. waiting until a new version is released upstream
2. Debian to pick it up (here we could move ahead if we want)
3. only then we are allowed to change the seeds in regard to the security ack

It will be done, but there are a few things not that much under our control we have to wait on.

Filed an upstream issue polling for a 2.0.3 release we could align to.
That is more proactive than just wait-and-see.
We can then decide our next steps based on the answer there.

Changed in htop:
status: Unknown → New

https://github.com/hishamhm/htop/issues/720 completed
2.1.0 released.

I opened Debian bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=889634 to poll if they would mind packing that soon.
If not we will likely go ahead for 18.04.

From Debian - looks good:
We're packaging it and should have it in sid by end of next week.
That should be well before the import sync freeze for Bionic Beaver
(planned for March 1st).

Changed in htop (Debian):
status: Unknown → New
Changed in htop:
status: New → Fix Released

Released in Debian (thanks) and also 2.1.0-2 is in Bionic universe now.
That said all preconditions for the MIR ack to be ok are complete.

I'll prep a seed change to review (IIRC this was only adding to main, not about shipping it on disk - let me know if intentions were otherwise)

Changed in htop (Ubuntu):
status: New → Triaged

Linked lp:~paelzer/ubuntu-seeds/18.04-support-htop which is the proposed seed change.

Changed in htop (Debian):
status: New → Fix Released

@MIR Team - we realized that this has no MIR Team ack yet, so I kindly ask you as well to formally ack this MIR.

MIR approved.

Changed in htop (Ubuntu):
status: Triaged → Fix Committed
Matthias Klose (doko) wrote :

this doesn't show up in any component mismatches

Matthias Klose (doko) wrote :

Override component to main
htop 2.1.0-2 in bionic: universe/utils -> main
htop 2.1.0-2 in bionic amd64: universe/utils/optional/100% -> main
htop 2.1.0-2 in bionic arm64: universe/utils/optional/100% -> main
htop 2.1.0-2 in bionic armhf: universe/utils/optional/100% -> main
htop 2.1.0-2 in bionic i386: universe/utils/optional/100% -> main
htop 2.1.0-2 in bionic ppc64el: universe/utils/optional/100% -> main
htop 2.1.0-2 in bionic s390x: universe/utils/optional/100% -> main
7 publications overridden.

Changed in htop (Ubuntu):
milestone: ubuntu-17.10 → none
status: Fix Committed → Fix Released

Thanks Mathias - I was waiting on an ack on https://code.launchpad.net/~paelzer/ubuntu-seeds/18.04-support-htop/+merge/337201 which will trigger the mismatch then.

Branch is merged now.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.