You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
147 lines
5.1 KiB
147 lines
5.1 KiB
7 years ago
|
Description: Fix unsafe extraction by using mkdir() instead of shell command
|
||
|
This commit fixes following vulnerabilities:
|
||
|
|
||
|
- CVE-2016-1243: stack buffer overflow caused by blindly trusting on
|
||
|
pathname lengths of archived files
|
||
|
|
||
|
Stack allocated buffer sysbuf was filled with sprintf() without any
|
||
|
bounds checking in extracTree() function.
|
||
|
|
||
|
- CVE-2016-1244: execution of unsanitized input
|
||
|
|
||
|
Shell command used for creating directory paths was constructed by
|
||
|
concatenating names of archived files to the end of the command
|
||
|
string.
|
||
|
|
||
|
So, if the user was tricked to extract a specially crafted .adf file,
|
||
|
the attacker was able to execute arbitrary code with privileges of the
|
||
|
user.
|
||
|
|
||
|
This commit fixes both issues by
|
||
|
|
||
|
1) replacing mkdir shell commands with mkdir() function calls
|
||
|
2) removing redundant sysbuf buffer
|
||
|
|
||
|
Author: Tuomas Räsänen <tuomasjjrasanen@tjjr.fi>
|
||
|
Last-Update: 2016-09-20
|
||
|
--
|
||
|
--- a/examples/unadf.c
|
||
|
+++ b/examples/unadf.c
|
||
|
@@ -24,6 +24,8 @@
|
||
|
|
||
|
#define UNADF_VERSION "1.0"
|
||
|
|
||
|
+#include <sys/stat.h>
|
||
|
+#include <sys/types.h>
|
||
|
|
||
|
#include<stdlib.h>
|
||
|
#include<errno.h>
|
||
|
@@ -31,17 +33,15 @@
|
||
|
|
||
|
#include "adflib.h"
|
||
|
|
||
|
-/* The portable way used to create a directory is to call the MKDIR command via the
|
||
|
- * system() function.
|
||
|
- * It is used to create the 'dir1' directory, like the 'dir1/dir11' directory
|
||
|
+/* The portable way used to create a directory is to call mkdir()
|
||
|
+ * which is defined by following standards: SVr4, BSD, POSIX.1-2001
|
||
|
+ * and POSIX.1-2008
|
||
|
*/
|
||
|
|
||
|
/* the portable way to check if a directory 'dir1' already exists i'm using is to
|
||
|
* do fopen('dir1','rb'). NULL is returned if 'dir1' doesn't exists yet, an handle instead
|
||
|
*/
|
||
|
|
||
|
-#define MKDIR "mkdir"
|
||
|
-
|
||
|
#ifdef WIN32
|
||
|
#define DIRSEP '\\'
|
||
|
#else
|
||
|
@@ -51,6 +51,13 @@
|
||
|
#define EXTBUFL 1024*8
|
||
|
|
||
|
|
||
|
+static void mkdirOrLogErr(const char *const path)
|
||
|
+{
|
||
|
+ if (mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO))
|
||
|
+ fprintf(stderr, "mkdir: cannot create directory '%s': %s\n",
|
||
|
+ path, strerror(errno));
|
||
|
+}
|
||
|
+
|
||
|
void help()
|
||
|
{
|
||
|
puts("unadf [-lrcsp -v n] dumpname.adf [files-with-path] [-d extractdir]");
|
||
|
@@ -152,7 +159,6 @@ void extractTree(struct Volume *vol, str
|
||
|
{
|
||
|
struct Entry* entry;
|
||
|
char *buf;
|
||
|
- char sysbuf[200];
|
||
|
|
||
|
while(tree) {
|
||
|
entry = (struct Entry*)tree->content;
|
||
|
@@ -162,16 +168,14 @@ void extractTree(struct Volume *vol, str
|
||
|
buf=(char*)malloc(strlen(path)+1+strlen(entry->name)+1);
|
||
|
if (!buf) return;
|
||
|
sprintf(buf,"%s%c%s",path,DIRSEP,entry->name);
|
||
|
- sprintf(sysbuf,"%s %s",MKDIR,buf);
|
||
|
if (!qflag) printf("x - %s%c\n",buf,DIRSEP);
|
||
|
+ if (!pflag) mkdirOrLogErr(buf);
|
||
|
}
|
||
|
else {
|
||
|
- sprintf(sysbuf,"%s %s",MKDIR,entry->name);
|
||
|
if (!qflag) printf("x - %s%c\n",entry->name,DIRSEP);
|
||
|
+ if (!pflag) mkdirOrLogErr(entry->name);
|
||
|
}
|
||
|
|
||
|
- if (!pflag) system(sysbuf);
|
||
|
-
|
||
|
if (tree->subdir!=NULL) {
|
||
|
if (adfChangeDir(vol,entry->name)==RC_OK) {
|
||
|
if (buf!=NULL)
|
||
|
@@ -301,21 +305,20 @@ void processFile(struct Volume *vol, cha
|
||
|
extractFile(vol, name, path, extbuf, pflag, qflag);
|
||
|
}
|
||
|
else {
|
||
|
- /* the all-in-one string : to call system(), to find the filename, the convert dir sep char ... */
|
||
|
- bigstr=(char*)malloc(strlen(MKDIR)+1+strlen(path)+1+strlen(name)+1);
|
||
|
+ bigstr=(char*)malloc(strlen(path)+1+strlen(name)+1);
|
||
|
if (!bigstr) { fprintf(stderr,"processFile : malloc"); return; }
|
||
|
|
||
|
/* to build to extract path */
|
||
|
if (strlen(path)>0) {
|
||
|
- sprintf(bigstr,"%s %s%c%s",MKDIR,path,DIRSEP,name);
|
||
|
- cdstr = bigstr+strlen(MKDIR)+1+strlen(path)+1;
|
||
|
+ sprintf(bigstr,"%s%c%s",path,DIRSEP,name);
|
||
|
+ cdstr = bigstr+strlen(path)+1;
|
||
|
}
|
||
|
else {
|
||
|
- sprintf(bigstr,"%s %s",MKDIR,name);
|
||
|
- cdstr = bigstr+strlen(MKDIR)+1;
|
||
|
+ sprintf(bigstr,"%s",name);
|
||
|
+ cdstr = bigstr;
|
||
|
}
|
||
|
/* the directory in which the file will be extracted */
|
||
|
- fullname = bigstr+strlen(MKDIR)+1;
|
||
|
+ fullname = bigstr;
|
||
|
|
||
|
/* finds the filename, and separates it from the path */
|
||
|
filename = strrchr(bigstr,'/')+1;
|
||
|
@@ -333,7 +336,7 @@ void processFile(struct Volume *vol, cha
|
||
|
return;
|
||
|
tfile = fopen(fullname,"r"); /* the only portable way to test if the dir exists */
|
||
|
if (tfile==NULL) { /* does't exist : create it */
|
||
|
- if (!pflag) system(bigstr);
|
||
|
+ if (!pflag) mkdirOrLogErr(bigstr);
|
||
|
if (!qflag) printf("x - %s%c\n",fullname,DIRSEP);
|
||
|
}
|
||
|
else
|
||
|
@@ -350,7 +353,7 @@ void processFile(struct Volume *vol, cha
|
||
|
return;
|
||
|
tfile = fopen(fullname,"r");
|
||
|
if (tfile==NULL) {
|
||
|
- if (!pflag) system(bigstr);
|
||
|
+ if (!pflag) mkdirOrLogErr(bigstr);
|
||
|
if (!qflag) printf("x - %s%c\n",fullname,DIRSEP);
|
||
|
}
|
||
|
else
|