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) => (unassigned) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1644364 Title: [MIR] htop To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/htop/+bug/1644364/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs