which-bugs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

path_clean_up buffer overflow


From: Tobias Stoeckmann
Subject: path_clean_up buffer overflow
Date: Sun, 21 Jun 2015 12:28:53 +0200

It is possible to overflow the global variable result in function
path_clean_up.

During concatenation of supplied path and working directory (or if
path is longer than 255 characters in general), no bound checks are
performed.

This could lead to a buffer overflow if working directory and path
are sufficiently long.

In addition to the boundary check, I changed the function's signature
to "const char". With this change, it is possible to simply return path
when no concatenation is needed. This leads to faster execution and no
boundary issues.

As with a previous mail, I recommend to use dynamically allocated memory
instead. With even more effort, the usage of memory could be further
reduced when chunks of the path are written directly to stdout.

--- which-2.21/which.c~ 2015-06-21 11:53:10.569495520 +0200
+++ which-2.21/which.c  2015-06-21 11:57:20.817300729 +0200
@@ -192,7 +192,7 @@
   }
 }
 
-static char *path_clean_up(const char *path)
+static const char *path_clean_up(const char *path)
 {
   static char result[256];
 
@@ -216,7 +216,14 @@
      * See 
http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap04.html#tag_04_11
      */
     if (!saw_slash || *p1 != '/' || (p1 == path + 1 && p1[1] != '/'))
+    {
+      if (p2 >= result + sizeof(result) - 1)
+      {
+       fprintf(stderr, "Can't create full path\n");
+       exit(-1);
+      }
       *p2++ = *p1;
+    }
     if (saw_slash_dot && (*p1 == '/'))
       p2 -= 2;
     if (saw_slash_dot_dot && (*p1 == '/'))
@@ -225,10 +232,7 @@
       do
       {
        if (--p2 < result)
-       {
-         strcpy(result, path);
-         return result;
-       }
+         return path;
        if (*p2 == '/')
          ++cnt;
       }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]