Skip to content

Restrict htoprc symlink resolution with owner check #1947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Explorer09 wants to merge 1 commit into
base: main
Choose a base branch
from
+69 −3
Open

Restrict htoprc symlink resolution with owner check #1947

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 69 additions & 3 deletions Settings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,74 @@ int Settings_write(const Settings* this, bool onCrash) {
return r;
}

static void Settings_resolveSymlink(char** resolvedPath, const char* path) {
int fd = -1;
int openFlags = O_NOCTTY | O_NOFOLLOW | O_NONBLOCK;
#ifdef O_EXEC
openFlags |= O_EXEC;
#else
// O_EXEC is not supported in Linux.
openFlags |= O_RDONLY;
#endif
#ifdef O_PATH
// O_PATH is specific to Linux and FreeBSD.
openFlags |= O_PATH;
#endif
do {
fd = open(path, openFlags);
} while (fd < 0 && errno == EINTR);
Comment on lines +811 to +817

@coderabbitai coderabbitai Bot Jun 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect supported targets and existing feature checks related to O_PATH/readlinkat.
# Expectation: every supported target either supports opening symlinks by fd plus readlinkat(fd, ""),
# or the PR adds an intentional platform-specific fallback/guard.

set -euo pipefail

printf '\n== Build/platform references ==\n'
rg -n -C3 '\b(linux|freebsd|openbsd|netbsd|darwin|macos|solaris|bsd)\b' \
   -g '!**/build/**' -g '!**/dist/**' -g '!**/node_modules/**' || true

printf '\n== Feature checks and direct usage ==\n'
rg -n -C4 '\b(O_PATH|O_EXEC|O_NOFOLLOW|readlinkat|realpath|lstat|readlink)\b' \
   -g '!**/build/**' -g '!**/dist/**' -g '!**/node_modules/**' || true

printf '\n== Candidate build/config files ==\n'
fd -a 'configure|configure.ac|meson.build|CMakeLists.txt|Makefile|ci|workflow|Dockerfile|README|INSTALL' . || true

Repository: htop-dev/htop

Length of output: 50369

🏁 Script executed:

cat -n Settings.c | sed -n '800,870p'

Repository: htop-dev/htop

Length of output: 2414

🏁 Script executed:

rg -n -A 5 -B 5 'readlinkat|O_PATH|O_EXEC|O_NOFOLLOW' Settings.c

Repository: htop-dev/htop

Length of output: 1345

🏁 Script executed:

rg -n '`#ifdef`|`#if` defined' Settings.c | head -30

Repository: htop-dev/htop

Length of output: 369

🏁 Script executed:

# Search for any readlinkat-related comments or conditionals that may indicate platform issues
rg -n 'readlinkat|empty.?path' --type-list | head -20

Repository: htop-dev/htop

Length of output: 447

🏁 Script executed:

# Check if there are any FreeBSD, OpenBSD, NetBSD, Darwin, or Solaris specific workarounds
rg -B5 -A5 'noPath|readlinkat' Settings.c

Repository: htop-dev/htop

Length of output: 1073

🏁 Script executed:

# Check build/configure for readlinkat checks
rg -n 'readlinkat|AC_CHECK' configure.ac | head -20

Repository: htop-dev/htop

Length of output: 1087

🌐 Web query:

readlinkat empty path "" support OpenBSD NetBSD Darwin Solaris

💡 Result:

Support for passing an empty string ("") as the path argument to readlinkat varies significantly across operating systems. On Linux, readlinkat explicitly supports an empty string path argument, provided that the AT_EMPTY_PATH flag is used or the kernel version is 2.6.39 or later [1][2][3][4]. In this specific case, the call operates directly on the symbolic link referred to by the file descriptor (dirfd), which must have been opened appropriately (typically with O_PATH and O_NOFOLLOW) [1][3]. Conversely, on OpenBSD, NetBSD, Darwin (macOS), and according to the POSIX standards, passing an empty string as a path argument to readlinkat is not supported and will result in an error [5][6][7][8][9]. POSIX specifically defines that readlinkat shall fail with ENOENT if the path argument is an empty string [9]. Similarly, these BSD-derived systems do not implement the Linux-specific AT_EMPTY_PATH extension, meaning standard path resolution rules apply where an empty string is treated as an invalid path name [5][6][10]. Summary: - Linux: Supported (requires specific flags/kernel version) [1][2][3]. - OpenBSD, NetBSD, Darwin, POSIX: Not supported (returns error, typically ENOENT) [5][6][7][8][9].

Citations:

Symlink resolution will fail silently on FreeBSD, OpenBSD, NetBSD, and Darwin.

The readlinkat(fd, "") call at line 840 is Linux-specific and unsupported on FreeBSD, OpenBSD, NetBSD, Darwin, and Solaris—it will return ENOENT. When this fails, the function falls back to returning an empty string (line 866), which breaks trusted symlink config resolution on those platforms without any diagnostic. Add #ifdef HAVE_DECL_READLINKAT_EMPTY_PATH or platform-specific guards to fall back to an alternative resolution method (e.g., readlink() if symlink was already opened with O_PATH) on non-Linux systems, or document this as a known limitation.

Evidence

Line 840 uses readlinkat(fd, "", buf, PATH_MAX) unconditionally. According to POSIX and BSD implementations, an empty string path argument is invalid and causes ENOENT. This is a Linux-specific extension not available on FreeBSD, OpenBSD, NetBSD, Darwin, or Solaris.


if (fd < 0)
goto noPath;
Comment thread
Explorer09 marked this conversation as resolved.

struct stat sb;
int err = fstat(fd, &sb);
if (err)
goto fileBroken;

if (!S_ISLNK(sb.st_mode) || (sb.st_uid != 0 && sb.st_uid != geteuid())) {
// Not a symbolic link or the symbolic link is not trusted.
// Return the path to the link itself. This allows the link
// target to be opened read-only when desirable.
close(fd);
*resolvedPath = xStrdup(path);
return;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

if (sb.st_size < 0 || sb.st_size >= PATH_MAX)
goto fileBroken;

char buf[PATH_MAX];
ssize_t len = readlinkat(fd, "", buf, PATH_MAX);
close(fd);

if (len < 0 || len > sb.st_size)
goto noPath;

buf[len] = '\0';
size_t dirnameLen = 0;
if (buf[0] != '/') {
const char *lastSlash = strrchr(path, '/');
dirnameLen = lastSlash ? (size_t)(lastSlash - (const char*)path + 1) : 0;
}
char* intermediatePath = xMalloc(dirnameLen + (size_t)len + 1);
memcpy(intermediatePath, path, dirnameLen);
memcpy(intermediatePath + dirnameLen, buf, (size_t)len + 1);
char* ptr = realpath(intermediatePath, buf);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
free(intermediatePath);
if (!ptr)
goto noPath;

*resolvedPath = xStrdup(buf);
return;

fileBroken:
close(fd);
noPath:
*resolvedPath = xStrdup("");
return;
}

Settings* Settings_new(const Machine* host, Hashtable* dynamicMeters, Hashtable* dynamicColumns, Hashtable* dynamicScreens) {
Settings* this = xCalloc(1, sizeof(Settings));

Expand Down Expand Up @@ -877,9 +945,7 @@ Settings* Settings_new(const Machine* host, Hashtable* dynamicMeters, Hashtable*
legacyDotfile = String_cat(home, "/.htoprc");
}

this->filename = xMalloc(PATH_MAX);
if (!realpath(this->initialFilename, this->filename))
free_and_xStrdup(&this->filename, this->initialFilename);
Settings_resolveSymlink(&this->filename, this->initialFilename);

this->colorScheme = 0;
#ifdef HAVE_GETMOUSE
Expand Down
Loading