Merge pull request #61 from maqifrnswa/buffer_overflows

Prevent buffer overflows from fixed path strings fixes #58

lib/helper.c

@@ -9,20 +9,52 @@
 #include <string.h>
 #include <errno.h>
 
+#ifdef _WIN32
+  #define realpath(N,R) _fullpath((R),(N),_MAX_PATH)
+  #include <direct.h>
+  #ifndef PATH_MAX
+    #define PATH_MAX _MAX_PATH
+  #endif
+#else
+  #include <limits.h>
+#endif
+
 #define VERBOSE 0
 
 FILE* unshield_fopen_for_reading(Unshield* unshield, int index, const char* suffix)
 {
   if (unshield && unshield->filename_pattern)
   {
     FILE* result = NULL;
-    char filename[256];
-    char dirname[256];
+    char* filename;
+    char* dirname;
     char * p = strrchr(unshield->filename_pattern, '/');
     const char *q;
     struct dirent *dent = NULL;
     DIR *sourcedir;
-    snprintf(filename, sizeof(filename), unshield->filename_pattern, index, suffix);
+    long int path_max;
+
+    #ifdef PATH_MAX
+    path_max = PATH_MAX;
+    #else
+    path_max = pathconf(prefix, _PC_PATH_MAX);
+    if (path_max <= 0)
+      path_max = 4096;
+    #endif
+
+    dirname = malloc(path_max);
+    filename = malloc(path_max);
+    if (filename == NULL || dirname == NULL)
+    {
+      unshield_error("Unable to allocate memory.\n");
+      goto exit;
+    }
+
+    if(snprintf(filename, path_max, unshield->filename_pattern, index, suffix)>=path_max)
+    {
+      unshield_error("Pathname exceeds system limits.\n");
+      goto exit;
+    }
     q=strrchr(filename,'/');
     if (q)
       q++;
@@ -31,11 +63,11 @@ FILE* unshield_fopen_for_reading(Unshield* unshield, int index, const char* suff
 
     if (p)
     {
-      strncpy( dirname, unshield->filename_pattern,sizeof(dirname));
-      if ((unsigned int)(p-unshield->filename_pattern) > sizeof(dirname))
+      strncpy( dirname, unshield->filename_pattern,path_max);
+      if ((unsigned int)(p-unshield->filename_pattern) > path_max)
       {
         unshield_trace("WARN: size\n");
-        dirname[sizeof(dirname)-1]=0;
+        dirname[path_max-1]=0;
       }
       else
         dirname[(p-unshield->filename_pattern)] = 0;
@@ -62,7 +94,11 @@ FILE* unshield_fopen_for_reading(Unshield* unshield, int index, const char* suff
         goto exit;
       }
       else
-        snprintf(filename, sizeof(filename), "%s/%s", dirname, dent->d_name);
+        if(snprintf(filename, path_max, "%s/%s", dirname, dent->d_name)>=path_max)
+        {
+          unshield_error("Pathname exceeds system limits.\n");
+          goto exit;
+        }
     }
     else
       unshield_trace("Could not open directory %s error %s\n", dirname, strerror(errno));
@@ -75,7 +111,8 @@ FILE* unshield_fopen_for_reading(Unshield* unshield, int index, const char* suff
 exit:
     if (sourcedir)
       closedir(sourcedir);
-
+    free(filename);
+    free(dirname);
     return result;
   }
 

src/unshield.c

@@ -40,6 +40,10 @@
   #include <limits.h>
 #endif
 
+#ifndef NAME_MAX
+  #define NAME_MAX FILENAME_MAX
+#endif
+
 typedef enum 
 {
   OVERWRITE_ASK,
@@ -118,6 +122,8 @@ static bool make_sure_directory_exists(const char* directory)/*{{{*/
         #endif
         {
           fprintf(stderr, "Failed to create directory %s\n", directory);
+          if(strlen(directory)>NAME_MAX)
+            fprintf(stderr, "Directory name must be less than %i characters\n", NAME_MAX+1);
           goto exit;
         }
       }
@@ -360,8 +366,8 @@ static bool handle_parameters(
 static bool extract_file(Unshield* unshield, const char* prefix, int index)
 {
   bool success;
-  char dirname[256];
-  char filename[256];
+  char* dirname;
+  char* filename;
   char* p;
   int directory = unshield_file_directory(unshield, index);
   long int path_max;
@@ -378,29 +384,62 @@ static bool extract_file(Unshield* unshield, const char* prefix, int index)
 
   real_output_directory = malloc(path_max);
   real_filename = malloc(path_max);
+  dirname = malloc(path_max);
+  filename = malloc(path_max);
   if (real_output_directory == NULL || real_filename == NULL)
   {
     fprintf(stderr,"Unable to allocate memory.");
     success=false;
     goto exit;
   }
 
-  strcpy(dirname, output_directory);
-  strcat(dirname, "/");
 
-  if (prefix && prefix[0])
+  if(strlen(output_directory) < path_max-1)
   {
-    strcat(dirname, prefix);
+    strncpy(dirname, output_directory,path_max-1);
+    if (path_max > 0)
+      dirname[path_max - 1]= '\0';
     strcat(dirname, "/");
   }
+  else
+  {
+    fprintf(stderr, "\nOutput directory exceeds maximum path length.\n");
+    success = false;
+    goto exit;
+  }
+
+
+  if (prefix && prefix[0])
+  {
+    if(strlen(dirname)+strlen(prefix) < path_max-1)
+    {
+      strcat(dirname, prefix);
+      strcat(dirname, "/");
+    }
+    else
+    {
+    fprintf(stderr, "\nOutput directory exceeds maximum path length.\n");
+    success = false;
+    goto exit;
+    }
+  }
 
   if (!junk_paths && directory >= 0)
   {
     const char* tmp = unshield_directory_name(unshield, directory);
     if (tmp && tmp[0])
     {
-      strcat(dirname, tmp);
-      strcat(dirname, "/");
+      if(strlen(dirname)+strlen(tmp) < path_max-1)
+      {
+        strcat(dirname, tmp);
+        strcat(dirname, "/");
+      }
+      else
+      {
+      fprintf(stderr, "\nOutput directory exceeds maximum path length.\n");
+      success = false;
+      goto exit;
+      }
     }
   }
 
@@ -448,7 +487,7 @@ static bool extract_file(Unshield* unshield, const char* prefix, int index)
 
   make_sure_directory_exists(dirname);
 
-  snprintf(filename, sizeof(filename), "%s%s", 
+  snprintf(filename, path_max, "%s%s", 
       dirname, unshield_file_name(unshield, index));
 
   for (p = filename + strlen(dirname); *p != '\0'; p++)
@@ -481,11 +520,8 @@ static bool extract_file(Unshield* unshield, const char* prefix, int index)
     fprintf(stderr, "\n\nExtraction failed.\n");
     fprintf(stderr, "Possible directory traversal attack for: %s\n", filename);
     fprintf(stderr, "To be placed at: %s\n\n", real_filename);
-    exit_status = 1;
     success = false;
-    free(real_filename);
-    free(real_output_directory);
-    return success;
+    goto exit;
   }
 
   printf("  extracting: %s\n", filename);
@@ -513,6 +549,8 @@ static bool extract_file(Unshield* unshield, const char* prefix, int index)
   }
   free(real_filename);
   free(real_output_directory);
+  free(dirname);
+  free(filename);
   return success;
 }
 
@@ -630,7 +668,7 @@ static int list_files_helper(Unshield* unshield, const char* prefix, int first,
 
   for (i = first; i <= last; i++)
   {
-    char dirname[256];
+    char dirname[4096];
 
     if (unshield_file_is_valid(unshield, i) && should_process_file(unshield, i))
     {