]> granicus.if.org Git - procps-ng/commitdiff
top: eliminate that potential vulnerability for TOCTOU
authorJim Warner <james.warner@comcast.net>
Sat, 7 Oct 2017 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@enc.com.au>
Sat, 14 Oct 2017 10:48:00 +0000 (21:48 +1100)
Initially, I was going to ignore that coverity warning
CID #177876. But, since top may be running SETUID it's
best if it can be avoided instead. The fix was simple.

We'll trade the access() call for a real fopen() call.
This time-of-check-time-of-use warning should go away.
------------------------------------------------------

When XDG support was originally introduced in top, the
author made a poor choice in access(). A real question
that needed asking was 'does the file exist'. However,
the question that was asked was 'can this real user ID
or this real group ID access the file'. Then, when the
fopen() is finally issued, top would use the effective
user ID or the effective group ID to access that file.

That's what opened the potential TOCTOU vulnerability,
which was important only if top was running SUID/SGID.

Signed-off-by: Jim Warner <james.warner@comcast.net>
top/top.c

index 178db183e5235929d7c3e96178cf6a19d37a1ecf..42ead5731a5edff9f057e0c3e3ba2476280f10c5 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -3188,7 +3188,7 @@ static void configs_read (void) {
       p_home = ".";
    snprintf(Rc_name, sizeof(Rc_name), "%s/.%src", p_home, Myname);
 
-   if (access(Rc_name, F_OK)) {
+   if (!(fp = fopen(Rc_name, "r"))) {
       p = getenv("XDG_CONFIG_HOME");
       // ensure the path we get is absolute, fallback otherwise.
       if (!p || p[0] != '/') {
@@ -3198,9 +3198,9 @@ static void configs_read (void) {
       snprintf(Rc_name, sizeof(Rc_name), "%s/procps", p);
       (void)mkdir(Rc_name, 0700);
       snprintf(Rc_name, sizeof(Rc_name), "%s/procps/%src", p, Myname);
+      fp = fopen(Rc_name, "r");
    }
 
-   fp = fopen(Rc_name, "r");
    if (fp) {
       int tmp_whole, tmp_fract;
       if (fgets(fbuf, sizeof(fbuf), fp))       // ignore eyecatcher