emo/procps
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

sysctl: Check path is under /proc/sys

Browse Source 
sysctl would try to read/write any path you gave it either on
the command line or configuration file. It would append /proc/sys
on the path but not check for any sneaky path traversal with ../

This commit means it first resolves all paths using realpath(3)
and then checks the path starts with "/proc/sys/"

At first I thought this might be a non-issue, but perhaps someone
could put a file into the sysctl configuration path and.. do
something? Anyway its a 8-line fix and makes things more correct.

References:
 #179

Signed-off-by: Craig Small <csmall@dropbear.xyz>
This commit is contained in:
Craig Small 2021-07-20 22:36:15 +10:00
parent 05a720fdba
commit f25d462166
2 changed files with 29 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
NEWS
View File

@ -9,6 +9,7 @@ procps-ng-NEXT
* ps: Add IO Accounting fields issue #184 * ps: Add IO Accounting fields issue #184
* ps: Add PSS and USS fields issue #112 * ps: Add PSS and USS fields issue #112
* slabtop: Don't combine d and o options issue #160 * slabtop: Don't combine d and o options issue #160
* sysctl: Check resolved path to be under /proc/sys issue #179
* top: exploit some library smaps_rollup provisions issue #112 * top: exploit some library smaps_rollup provisions issue #112
* top: added four new IO accounting fields issue #184 * top: added four new IO accounting fields issue #184

28
sysctl.c
View File

@ -73,6 +73,24 @@ static size_t iolen = LINELEN;
static int pattern_match(const char *string, const char *pat); static int pattern_match(const char *string, const char *pat);
static int DisplayAll(const char *restrict const path); static int DisplayAll(const char *restrict const path);
static inline bool is_proc_path(
const char *path)
{
char *resolved_path;
if ( (resolved_path = realpath(path, NULL)) == NULL)
return false;
if (strncmp(PROC_PATH, resolved_path, strlen(PROC_PATH)) == 0) {
free(resolved_path);
return true;
}
xwarnx(_("Path is not under %s: %s"), PROC_PATH, path);
free(resolved_path);
return false;
}
static void slashdot(char *restrict p, char old, char new) static void slashdot(char *restrict p, char old, char new)
{ {
int warned = 1; int warned = 1;
@ -202,6 +220,11 @@ static int ReadSetting(const char *restrict const name)
if ((ts.st_mode & S_IRUSR) == 0) if ((ts.st_mode & S_IRUSR) == 0)
goto out; goto out;
if (!is_proc_path(tmpname)) {
rc = -1;
goto out;
}
if (S_ISDIR(ts.st_mode)) { if (S_ISDIR(ts.st_mode)) {
size_t len; size_t len;
len = strlen(tmpname); len = strlen(tmpname);
@ -432,6 +455,11 @@ static int WriteSetting(const char *setting)
goto out; goto out;
} }
if (!is_proc_path(tmpname)) {
rc = -1;
goto out;
}
if ((ts.st_mode & S_IWUSR) == 0) { if ((ts.st_mode & S_IWUSR) == 0) {
xwarn(_("setting key \"%s\""), outname); xwarn(_("setting key \"%s\""), outname);
goto out; goto out;