Skip to content

Commit 8e973d7

Browse files
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
1 parent 7a01944 commit 8e973d7

File tree

1 file changed

+22
-19
lines changed

1 file changed

+22
-19
lines changed

examples/unadf.c

+22-19
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,24 @@
2424

2525
#define UNADF_VERSION "1.0"
2626

27+
#include <sys/stat.h>
28+
#include <sys/types.h>
2729

2830
#include<stdlib.h>
2931
#include<errno.h>
3032
#include<string.h>
3133

3234
#include "adflib.h"
3335

34-
/* The portable way used to create a directory is to call the MKDIR command via the
35-
* system() function.
36-
* It is used to create the 'dir1' directory, like the 'dir1/dir11' directory
36+
/* The portable way used to create a directory is to call mkdir()
37+
* which is defined by following standards: SVr4, BSD, POSIX.1-2001
38+
* and POSIX.1-2008
3739
*/
3840

3941
/* the portable way to check if a directory 'dir1' already exists i'm using is to
4042
* do fopen('dir1','rb'). NULL is returned if 'dir1' doesn't exists yet, an handle instead
4143
*/
4244

43-
#define MKDIR "mkdir"
44-
4545
#ifdef WIN32
4646
#define DIRSEP '\\'
4747
#else
@@ -51,6 +51,13 @@
5151
#define EXTBUFL 1024*8
5252

5353

54+
static void mkdirOrLogErr(const char *const path)
55+
{
56+
if (mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO))
57+
fprintf(stderr, "mkdir: cannot create directory '%s': %s\n",
58+
path, strerror(errno));
59+
}
60+
5461
void help()
5562
{
5663
puts("unadf [-lrcsp -v n] dumpname.adf [files-with-path] [-d extractdir]");
@@ -154,7 +161,6 @@ void extractTree(struct Volume *vol, struct List* tree, char *path, unsigned cha
154161
{
155162
struct Entry* entry;
156163
char *buf;
157-
char sysbuf[200];
158164

159165
while(tree) {
160166
entry = (struct Entry*)tree->content;
@@ -164,16 +170,14 @@ void extractTree(struct Volume *vol, struct List* tree, char *path, unsigned cha
164170
buf=(char*)malloc(strlen(path)+1+strlen(entry->name)+1);
165171
if (!buf) return;
166172
sprintf(buf,"%s%c%s",path,DIRSEP,entry->name);
167-
sprintf(sysbuf,"%s %s",MKDIR,buf);
168173
if (!qflag) printf("x - %s%c\n",buf,DIRSEP);
174+
if (!pflag) mkdirOrLogErr(buf);
169175
}
170176
else {
171-
sprintf(sysbuf,"%s %s",MKDIR,entry->name);
172177
if (!qflag) printf("x - %s%c\n",entry->name,DIRSEP);
178+
if (!pflag) mkdirOrLogErr(entry->name);
173179
}
174180

175-
if (!pflag) system(sysbuf);
176-
177181
if (tree->subdir!=NULL) {
178182
if (adfChangeDir(vol,entry->name)==RC_OK) {
179183
if (buf!=NULL)
@@ -304,21 +308,20 @@ void processFile(struct Volume *vol, char* name, char* path, unsigned char *extb
304308
extractFile(vol, name, path, extbuf, pflag, qflag);
305309
}
306310
else {
307-
/* the all-in-one string : to call system(), to find the filename, the convert dir sep char ... */
308-
bigstr=(char*)malloc(strlen(MKDIR)+1+strlen(path)+1+strlen(name)+1);
311+
bigstr=(char*)malloc(strlen(path)+1+strlen(name)+1);
309312
if (!bigstr) { fprintf(stderr,"processFile : malloc"); return; }
310313

311314
/* to build to extract path */
312315
if (strlen(path)>0) {
313-
sprintf(bigstr,"%s %s%c%s",MKDIR,path,DIRSEP,name);
314-
cdstr = bigstr+strlen(MKDIR)+1+strlen(path)+1;
316+
sprintf(bigstr,"%s%c%s",path,DIRSEP,name);
317+
cdstr = bigstr+strlen(path)+1;
315318
}
316319
else {
317-
sprintf(bigstr,"%s %s",MKDIR,name);
318-
cdstr = bigstr+strlen(MKDIR)+1;
320+
sprintf(bigstr,"%s",name);
321+
cdstr = bigstr;
319322
}
320323
/* the directory in which the file will be extracted */
321-
fullname = bigstr+strlen(MKDIR)+1;
324+
fullname = bigstr;
322325

323326
/* finds the filename, and separates it from the path */
324327
filename = strrchr(bigstr,'/')+1;
@@ -336,7 +339,7 @@ void processFile(struct Volume *vol, char* name, char* path, unsigned char *extb
336339
return;
337340
tfile = fopen(fullname,"r"); /* the only portable way to test if the dir exists */
338341
if (tfile==NULL) { /* does't exist : create it */
339-
if (!pflag) system(bigstr);
342+
if (!pflag) mkdirOrLogErr(bigstr);
340343
if (!qflag) printf("x - %s%c\n",fullname,DIRSEP);
341344
}
342345
else
@@ -353,7 +356,7 @@ void processFile(struct Volume *vol, char* name, char* path, unsigned char *extb
353356
return;
354357
tfile = fopen(fullname,"r");
355358
if (tfile==NULL) {
356-
if (!pflag) system(bigstr);
359+
if (!pflag) mkdirOrLogErr(bigstr);
357360
if (!qflag) printf("x - %s%c\n",fullname,DIRSEP);
358361
}
359362
else

0 commit comments

Comments
 (0)