]> granicus.if.org Git - procps-ng/commitdiff
0055-ps/display.c: Harden show_tree().
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Sat, 9 Jun 2018 11:45:38 +0000 (21:45 +1000)
1/ Do not go deeper than the size of forest_prefix[], to prevent a
buffer overflow (sizeof(forest_prefix) is roughly 128K, but the maximum
/proc/sys/kernel/pid_max is 4M). (actually, we go deeper, but we stop
adding bytes to forest_prefix[])

2/ Always null-terminate forest_prefix[] at the current level.

---------------------------- adapted for newlib branch
. logic is quite different with 'stacks' vs. 'proc_t'
. a commented out 'debug' line was no longer present

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

index b87de648b9c49d35ca352704c1c22d61054df219..5cfee84b6288e6f8535885d2baa251b250b0dd55 100644 (file)
@@ -376,14 +376,22 @@ static void show_proc_array(int n){
 /***** show tree */
 /* this needs some optimization work */
 #define ADOPTED(x) 1
+
+#define IS_LEVEL_SAFE(level) \
+  ((level) >= 0 && (size_t)(level) < sizeof(forest_prefix))
+
 static void show_tree(const int self, const int n, const int level, const int have_sibling){
   int i = 0;
+
+  if(!IS_LEVEL_SAFE(level))
+    catastrophic_failure(__FILE__, __LINE__, _("please report this bug"));
+
   if(level){
     /* add prefix of "+" or "L" */
     if(have_sibling) forest_prefix[level-1] = '+';
     else             forest_prefix[level-1] = 'L';
-    forest_prefix[level] = '\0';
   }
+  forest_prefix[level] = '\0';
   show_one_proc(processes[self],format_list);  /* first show self */
   for(;;){  /* look for children */
     if(i >= n) return; /* no children */
@@ -394,8 +402,8 @@ static void show_tree(const int self, const int n, const int level, const int ha
     /* change our prefix to "|" or " " for the children */
     if(have_sibling) forest_prefix[level-1] = '|';
     else             forest_prefix[level-1] = ' ';
-    forest_prefix[level] = '\0';
   }
+  forest_prefix[level] = '\0';
   for(;;){
     int self_pid;
     int more_children = 1;
@@ -406,15 +414,18 @@ static void show_tree(const int self, const int n, const int level, const int ha
     else
       if(rSv(ID_PPID, s_int, processes[i+1]) != self_pid) more_children = 0;
     if(self_pid==1 && ADOPTED(processes[i]) && forest_type!='u')
-      show_tree(i++, n, level,   more_children);
+      show_tree(i++, n, level, more_children);
     else
-      show_tree(i++, n, level+1, more_children);
+      show_tree(i++, n, IS_LEVEL_SAFE(level+1) ? level+1 : level, more_children);
     if(!more_children) break;
   }
   /* chop prefix that children added -- do we need this? */
+  /* chop prefix that children added */
   forest_prefix[level] = '\0';
 }
 
+#undef IS_LEVEL_SAFE
+
 /***** show forest */
 static void show_forest(const int n){
   int i = n;