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

Bug#774527: Patches fixing arc: directory traversal



Hi,

Attached are 3 patches fixing this, apply order:

arc-5.21p-hdrv1-read-fix.patch
arc-5.21p-fix-arcdie.patch
arc-5.21p-directory-traversel.patch

The first patch really is an unrelated issue I noticed while working on
this, the second patch is a preparation patch and the third patch is
the real fix.

Regards,

Hans

Fix directory traversal bugs

arc archives do not contain directory hierarchies, only filenames, so refuse
to operate on archives which have the directory-seperator inside filenames.

BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=774527
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1179143
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
diff -up arc-5.21p/arcio.c~ arc-5.21p/arcio.c
--- arc-5.21p/arcio.c~	2015-01-16 13:04:16.000000000 +0100
+++ arc-5.21p/arcio.c	2015-01-16 15:45:31.389010626 +0100
@@ -109,6 +109,9 @@ readhdr(hdr, f)			/* read a header from
 #if	_MTS
 	(void) atoe(hdr->name, strlen(hdr->name));
 #endif
+	if (strchr(hdr->name, CUTOFF) != NULL)
+		arcdie("%s contains illegal filename %s", arcname, hdr->name);
+
 	for (i = 0, hdr->size=0; i<4; hdr->size<<=8, hdr->size += dummy[16-i], i++);
 	hdr->date = (short) ((dummy[18] << 8) + dummy[17]);
 	hdr->time = (short) ((dummy[20] << 8) + dummy[19]);
Fix arcdie crash when called with more then 1 variable argument

Add proper vararg handling to fix crash on 64 bit machines when arcdie gets
called with more then 1 variable argument.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
diff -up arc-5.21p/arcmisc.c~ arc-5.21p/arcmisc.c
--- arc-5.21p/arcmisc.c~	2010-08-07 15:06:42.000000000 +0200
+++ arc-5.21p/arcmisc.c	2015-01-16 16:10:29.322603290 +0100
@@ -4,6 +4,7 @@
  */
 
 #include <stdio.h>
+#include <stdarg.h>
 #include <ctype.h>
 #include "arc.h"
 
@@ -223,11 +224,13 @@ upper(string)
 }
 /* VARARGS1 */
 VOID
-arcdie(s, arg1, arg2, arg3)
-	char           *s;
+arcdie(const char *s, ...)
 {
+	va_list args;
 	fprintf(stderr, "ARC: ");
-	fprintf(stderr, s, arg1, arg2, arg3);
+	va_start(args, s);
+	vfprintf(stderr, s, args);
+	va_end(args);
 	fprintf(stderr, "\n");
 #if	UNIX
 	perror("UNIX");
Fix version 1 arc header reading

The code for v1 hdr reading was reading the packed header directly into an
unpacked struct.

Use the same read to dummy array, then manual unpack to header struct as
used for v2 headers for v1 headers too.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
diff -ur arc-5.21p/arcio.c arc-5.21p.new/arcio.c
--- arc-5.21p/arcio.c	2010-08-07 15:06:42.000000000 +0200
+++ arc-5.21p.new/arcio.c	2015-01-16 12:59:43.203289118 +0100
@@ -37,6 +37,7 @@
 #endif
 	char            name[FNLEN];	/* filename buffer */
 	int             try = 0;/* retry counter */
+	int             hdrlen;
 	static int      first = 1;	/* true only on first read */
 
 	if (!f)			/* if archive didn't open */
@@ -92,23 +93,19 @@
 		printf("I think you need a newer version of ARC.\n");
 		exit(1);
 	}
+
 	/* amount to read depends on header type */
+	if (hdrver == 1) {
+		hdrlen = 23; /* old style is shorter */
+	} else {
+		hdrlen = 27;
+	}
 
-	if (hdrver == 1) {	/* old style is shorter */
-		if (fread(hdr, sizeof(struct heads) - sizeof(long int), 1, f) != 1)
-			arcdie("%s was truncated", arcname);
-		hdrver = 2;	/* convert header to new format */
-		hdr->length = hdr->size;	/* size is same when not
-						 * packed */
-	} else
-#if	MSDOS
-		if (fread(hdr, sizeof(struct heads), 1, f) != 1)
-			arcdie("%s was truncated", arcname);
-#else
-		if (fread(dummy, 27, 1, f) != 1)
-			arcdie("%s was truncated", arcname);
+	if (fread(dummy, hdrlen, 1, f) != 1)
+		arcdie("%s was truncated", arcname);
 
 	for (i = 0; i < FNLEN; hdr->name[i] = dummy[i], i++);
+	hdr->name[FNLEN - 1] = 0; /* ensure 0 termination */
 #if	_MTS
 	(void) atoe(hdr->name, strlen(hdr->name));
 #endif
@@ -116,8 +113,14 @@
 	hdr->date = (short) ((dummy[18] << 8) + dummy[17]);
 	hdr->time = (short) ((dummy[20] << 8) + dummy[19]);
 	hdr->crc = (short) ((dummy[22] << 8) + dummy[21]);
-	for (i = 0, hdr->length=0; i<4; hdr->length<<=8, hdr->length += dummy[26-i], i++);
-#endif
+
+	if (hdrver == 1) {
+		hdrver = 2;	/* convert header to new format */
+		hdr->length = hdr->size;	/* size is same when not
+						 * packed */
+	} else {
+		for (i = 0, hdr->length=0; i<4; hdr->length<<=8, hdr->length += dummy[26-i], i++);
+	}
 
 	if (hdr->date > olddate
 	    || (hdr->date == olddate && hdr->time > oldtime)) {

Reply to: