htop: fix possible integer overflow in format strings - #2056
Conversation
%lf usage is redundant
📝 WalkthroughWalkthroughThe changes correct format specifiers across CPU labels, user IDs, TTY paths, file descriptor meters, and pressure meters. CPU, user, and TTY values use unsigned decimal formatting. File descriptor and pressure values use float formatting. Unlimited file descriptor values remain displayed as Suggested reviewers: Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 393e4d3f-e2de-4570-8c1d-0b7dfea94138
📒 Files selected for processing (6)
AffinityPanel.cAvailableMetersPanel.cFileDescriptorMeter.cProcess.clinux/LinuxProcessTable.clinux/PressureStallMeter.c
| } | ||
| break; | ||
| case ST_UID: xSnprintf(buffer, n, "%*d ", Process_uidDigits, this->st_uid); break; | ||
| case ST_UID: xSnprintf(buffer, n, "%*u ", Process_uidDigits, this->st_uid); break; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 '\buid_t\b|\bST_UID\b' --glob '*.[ch]' .Repository: htop-dev/htop
Length of output: 16019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant uid_t/user setup files:\n'
rg -n 'typedef|_uid_t|uid_t|UserId|getpwnam|getpwuid|ProcessTable_updateUid|UserTable' --glob '*.[ch]' .
printf '\nProcess.h context:\n'
sed -n '1,180p' Process.h
printf '\nRow.c width/print context:\n'
sed -n '1,175p' Row.c
printf '\nlibc/platform uid_t declarations and Process.c user handling:\n'
sed -n '1,120p' LinuxProcess.c
sed -n '1,120p' FreeBSDProcess.c
sed -n '1,120p' SolarisProcess.c
sed -n '1,120p' OpenBSDProcess.c
sed -n '1,120p' NetBSDProcess.c
sed -n '1,120p' DarwinProcess.c
sed -n '1,120p' DragonFlyBSDProcess.c
sed -n '1,120p' PCPProcess.c
sed -n '1,120p' UnsupportedProcess.c
printf '\nProcess.c UID formatting/compare/filter context:\n'
sed -n '730,815p' Process.c
sed -n '860,910p' Process.c
sed -n '1005,1035p' Process.cRepository: htop-dev/htop
Length of output: 25794
🌐 Web query:
POSIX uid_t format specifier printf %u unsigned int
💡 Result:
There is no specific POSIX printf format specifier for uid_t [1]. POSIX defines uid_t as an integer type, but it does not mandate whether it is signed or unsigned, nor does it define a specific width (e.g., 32-bit or 64-bit) [2][3][4]. Because of this, using a single format specifier like %u is not portable across all POSIX-compliant systems [3]. To print a uid_t portably, the recommended approach is to cast the value to a known, larger integer type that has a defined format specifier [1]. A common and safe practice is to cast it to (unsigned long) and use the %lu format specifier, as uid_t is guaranteed to be an integer type [1][2]. Example of portable usage: printf("User ID: %lu\n", (unsigned long)my_uid); This avoids potential issues where uid_t might be a signed type on some platforms (where %u could behave unexpectedly) or wider than a standard int [3][5].
Citations:
- 1: https://www.austingroupbugs.net/view.php?id=576
- 2: https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/basedefs/sys_types.h.html
- 3: https://stackoverflow.com/questions/21370094/is-the-uid-t-type-signed-or-unsigned
- 4: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
- 5: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg480310.html
Use a guaranteed-width format for UID output.
Process.c prints this->st_uid with %u, but POSIX only defines uid_t as an integer type and does not guarantee that it matches unsigned int. Cast it to a portable type such as unsigned long and use %lu, or a width-specific format that preserves the maximum uid_t range.
%lf usage is redundant