Hi,
Seth Arnold of the Ubuntu security team did a short security audit of
the code and found a few things they didn't like:
https://bugs.launchpad.net/ubuntu/+source/htop/+bug/1644364
Quoting from there:
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)
Kind regards,
Daniel
--------------------------------
htop mailing list
Project website: http://hisham.hm/htop/
List options (unsubscribe, etc:) //www.freelists.org/list/htop/
--------------------------------
