From f25d462166f80b844d33dad3e4c06088c809a426 Mon Sep 17 00:00:00 2001
From: Craig Small <csmall@dropbear.xyz>
Date: Tue, 20 Jul 2021 22:36:15 +1000
Subject: [PATCH] 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>
---
 NEWS     |  1 +
 sysctl.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/NEWS b/NEWS
index 4a10be67..13543817 100644
--- 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 1a07bb5e..d6173f26 100644
--- 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;