[MIR] htop

[Joshua Powers]

[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)]

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?

[Joshua Powers (powersj)]

Yes - I have updated the section.

[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!

[Joshua Powers (powersj)]

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.

[Mathieu Trudel-Lapierre (cyphermox)]

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.

[Emily Ratliff (emilyr)]

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)]

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

[Seth Arnold (seth-arnold)]

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

[Joshua Powers (powersj)]

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.

[Seth Arnold (seth-arnold)]

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)]

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)]

Thanks Seth! Marking bug new again.

[Christian Ehrhardt  (paelzer)]

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.

[Christian Ehrhardt  (paelzer)]

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.

[Christian Ehrhardt  (paelzer)]

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.

[Christian Ehrhardt  (paelzer)]

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).

[Christian Ehrhardt  (paelzer)]

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)

[Christian Ehrhardt  (paelzer)]

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

[Christian Ehrhardt  (paelzer)]

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

[Mathieu Trudel-Lapierre (cyphermox)]

MIR approved.

[Matthias Klose (doko)]

this doesn't show up in any component mismatches

[Matthias Klose (doko)]

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.

[Christian Ehrhardt  (paelzer)]

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.

[Christian Ehrhardt  (paelzer)]

Branch is merged now.