sysctl: Check path is under /proc/sys

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>

diff --git a/NEWS b/NEWS
index 4a10be67052d30ed7b40d2845a954af82bf6b0f2..13543817c04d42e87d0818351b1ebb021130f857 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,7 @@ procps-ng-NEXT
   * ps: Add IO Accounting fields                           issue #184
   * ps: Add PSS and USS fields                             issue #112
   * 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: added four new IO accounting fields               issue #184
 

diff --git a/sysctl.c b/sysctl.c
index 1a07bb5e0c710227aa43781c59dbdd6a251df62e..d6173f26d2fe5ca03a42d7f6a7577619b2cae492 100644 (file)
--- a/sysctl.c
+++ b/sysctl.c
@@ -73,6 +73,24 @@ static size_t iolen = LINELEN;
 static int pattern_match(const char *string, const char *pat);
 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)
 {
        int warned = 1;
@@ -202,6 +220,11 @@ static int ReadSetting(const char *restrict const name)
        if ((ts.st_mode & S_IRUSR) == 0)
                goto out;
 
+       if (!is_proc_path(tmpname)) {
+           rc = -1;
+           goto out;
+       }
+
        if (S_ISDIR(ts.st_mode)) {
                size_t len;
                len = strlen(tmpname);
@@ -432,6 +455,11 @@ static int WriteSetting(const char *setting)
                goto out;
        }
 
+       if (!is_proc_path(tmpname)) {
+           rc = -1;
+           goto out;
+       }
+
        if ((ts.st_mode & S_IWUSR) == 0) {
                xwarn(_("setting key \"%s\""), outname);
                goto out;