top: restore configuration file backward compatibility

The Debian bug referenced below has nothing to do with
locales. In fact, top was made locale independent back
in release 3.3.13 (April, 2018). However, that bug did
reveal some misplaced logic which this patch corrects.

Prompted by the Qualys audit, all rcfile field strings
were checked for potential duplicates which could only
have resulted from some user's manual/malicious edits.

Unfortunately, that code was executed before top had a
chance to enforce the proper/maximum string length (in
the event an extremely old rcfile had just been read).
This created some potential string overrun references.

In top's original 3.3.15 implementation, the potential
overrun extended for 15 characters. That is the number
of field characters added with 3.3.9 (December, 2013).
But, since strchr() was used, no error exit was taken.

In the revised 3.3.16 implementation, the strchr() was
replaced with '&w->rc.fieldscur[n]'. This held overrun
to a single position while producing an error message.

So, this commit just moves that logic to a point where
fieldscur is guaranteed to be longer than EU_MAXPFLGS.

Reference(s):
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=951335
. revised 3.3.16 validation logic
commit 291d98ee5036567f93d21bc11142b0a7e2ee70ae
. original 3.3.15 validation logic
commit fdb58974e24c025a1f866f324c62f1d8f96234f8

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

diff --git a/NEWS b/NEWS
index 4e0e0f9d23b5a60f8c685313317d5bec210faa3c..a6001beac8f32da1edc7ae866c2b52a1349c19bc 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,7 @@
 procps-ng NEXT
 --------------
   * pgrep: Check sanity of SG_ARG_MAX                      issue #152
+  * top: ensure config file backward compatibility         Debian #951335
 
 procps-ng-3.3.16
 ----------------

diff --git a/top/top.c b/top/top.c
index 63ec5fe0fcc11529c68050a6c4003e4b6c799f5b..b4fe21e416ff3bb003e6ba89c043047e4f3c73de 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -3939,11 +3939,6 @@ static const char *configs_file (FILE *fp, const char *name, float *delay) {
  // too bad fscanf is not as flexible with his format string as snprintf
  #error Hey, fix the above fscanf 'PFLAGSSIZ' dependency !
 #endif
-      // ensure there's been no manual alteration of fieldscur
-      for (n = 0 ; n < EU_MAXPFLGS; n++) {
-         if (&w->rc.fieldscur[n] != strrchr(w->rc.fieldscur, w->rc.fieldscur[n]))
-            return p;
-      }
       // be tolerant of missing release 3.3.10 graph modes additions
       if (3 > fscanf(fp, "\twinflags=%d, sortindx=%d, maxtasks=%d, graph_cpus=%d, graph_mems=%d\n"
          , &w->rc.winflags, &w->rc.sortindx, &w->rc.maxtasks, &w->rc.graph_cpus, &w->rc.graph_mems))
@@ -3989,6 +3984,11 @@ static const char *configs_file (FILE *fp, const char *name, float *delay) {
                   return p;
             break;
       }
+      // ensure there's been no manual alteration of fieldscur
+      for (n = 0 ; n < EU_MAXPFLGS; n++) {
+         if (&w->rc.fieldscur[n] != strrchr(w->rc.fieldscur, w->rc.fieldscur[n]))
+            return p;
+      }
 #ifndef USE_X_COLHDR
       OFFw(w, NOHIFND_xxx | NOHISEL_xxx);
 #endif