CVE-2014-8117: Resource Management Errors (CWE-399)
value: softmagic.c in file before 5.21 does not properly limit recursion, which allows remote attackers to cause a denial of service (CPU consumption or crash) via unspecified vectors.
CWE description: Weaknesses in this category are related to improper management of system resources.
Published: 2014-12-17T19:59Z
Repository: https://github.com/file/file
Language: C
Files changed: src/file.h, src/funcs.c, src/softmagic.c
Commit date: 2014-11-23 13:54:27 +0000
Commit message: - reduce recursion level from 20 to 10 and make a symbolic constant for it.
- pull out the guts of saving and restoring the output buffer into functions
  and take care not to overwrite the error message if an error happened.

Diff: diff --git a/src/file.h b/src/file.h
index fdbc889d9..fa0f616db 100644
--- a/src/file.h
+++ b/src/file.h
@@ -25,11 +25,11 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
 /*
  * file.h - definitions for file(1) program
- * @(#)$File: file.h,v 1.154 2014/09/10 18:41:51 christos Exp $
+ * @(#)$File: file.h,v 1.155 2014/10/11 15:03:16 christos Exp $
  */
 
 #ifndef __file_h__
 #define __file_h__
 
@@ -493,10 +493,18 @@ protected int file_regcomp(file_regex_t *, const char *, int);
 protected int file_regexec(file_regex_t *, const char *, size_t, regmatch_t *,
     int);
 protected void file_regfree(file_regex_t *);
 protected void file_regerror(file_regex_t *, int, struct magic_set *);
 
+typedef struct {
+	char *buf;
+	uint32_t offset;
+} file_pushbuf_t;
+
+protected file_pushbuf_t *file_push_buffer(struct magic_set *);
+protected char  *file_pop_buffer(struct magic_set *, file_pushbuf_t *);
+
 #ifndef COMPILE_ONLY
 extern const char *file_names[];
 extern const size_t file_nnames;
 #endif
 
diff --git a/src/funcs.c b/src/funcs.c
index 1517a79f5..7b87dcfb8 100644
--- a/src/funcs.c
+++ b/src/funcs.c
@@ -25,11 +25,11 @@
  * SUCH DAMAGE.
  */
 #include "file.h"
 
 #ifndef	lint
-FILE_RCSID("@(#)$File: funcs.c,v 1.72 2014/05/14 23:15:42 christos Exp $")
+FILE_RCSID("@(#)$File: funcs.c,v 1.73 2014/09/10 18:41:51 christos Exp $")
 #endif	/* lint */
 
 #include "magic.h"
 #include <assert.h>
 #include <stdarg.h>
@@ -489,5 +489,45 @@ file_regerror(file_regex_t *rx, int rc, struct magic_set *ms)
 
 	(void)regerror(rc, &rx->rx, errmsg, sizeof(errmsg));
 	file_magerror(ms, "regex error %d for `%s', (%s)", rc, rx->pat,
 	    errmsg);
 }
+
+protected file_pushbuf_t *
+file_push_buffer(struct magic_set *ms)
+{
+	file_pushbuf_t *pb;
+
+	if (ms->event_flags & EVENT_HAD_ERR)
+		return NULL;
+
+	if ((pb = (CAST(file_pushbuf_t *, malloc(sizeof(*pb))))) == NULL)
+		return NULL;
+
+	pb->buf = ms->o.buf;
+	pb->offset = ms->offset;
+
+	ms->o.buf = NULL;
+	ms->offset = 0;
+
+	return pb;
+}
+
+protected char *
+file_pop_buffer(struct magic_set *ms, file_pushbuf_t *pb)
+{
+	char *rbuf;
+
+	if (ms->event_flags & EVENT_HAD_ERR) {
+		free(pb->buf);
+		free(pb);
+		return NULL;
+	}
+
+	rbuf = ms->o.buf;
+
+	ms->o.buf = pb->buf;
+	ms->offset = pb->offset;
+
+	free(pb);
+	return rbuf;
+}
diff --git a/src/softmagic.c b/src/softmagic.c
index 2b15f2c39..15dfce59c 100644
--- a/src/softmagic.c
+++ b/src/softmagic.c
@@ -30,11 +30,11 @@
  */
 
 #include "file.h"
 
 #ifndef	lint
-FILE_RCSID("@(#)$File: softmagic.c,v 1.196 2014/11/07 15:24:14 christos Exp $")
+FILE_RCSID("@(#)$File: softmagic.c,v 1.197 2014/11/11 17:48:23 christos Exp $")
 #endif	/* lint */
 
 #include "magic.h"
 #include <assert.h>
 #include <string.h>
@@ -61,10 +61,13 @@ private void cvt_8(union VALUETYPE *, const struct magic *);
 private void cvt_16(union VALUETYPE *, const struct magic *);
 private void cvt_32(union VALUETYPE *, const struct magic *);
 private void cvt_64(union VALUETYPE *, const struct magic *);
 
 #define OFFSET_OOB(n, o, i)	((n) < (o) || (i) > ((n) - (o)))
+
+#define MAX_RECURSION_LEVEL	10
+
 /*
  * softmagic - lookup one file in parsed, in-memory copy of database
  * Passed the name and FILE * of one file to be typed.
  */
 /*ARGSUSED1*/		/* nbytes passed for regularity, maybe need later */
@@ -1215,18 +1218,19 @@ private int
 mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
     size_t nbytes, size_t o, unsigned int cont_level, int mode, int text,
     int flip, int recursion_level, int *printed_something,
     int *need_separator, int *returnval)
 {
-	uint32_t soffset, offset = ms->offset;
+	uint32_t offset = ms->offset;
 	uint32_t lhs;
+	file_pushbuf_t *pb;
 	int rv, oneed_separator, in_type;
-	char *sbuf, *rbuf;
+	char *rbuf;
 	union VALUETYPE *p = &ms->ms_value;
 	struct mlist ml;
 
-	if (recursion_level >= 20) {
+	if (recursion_level >= MAX_RECURSION_LEVEL) {
 		file_error(ms, 0, "recursion nesting exceeded");
 		return -1;
 	}
 
 	if (mcopy(ms, p, m->type, m->flag & INDIR, s, (uint32_t)(offset + o),
@@ -1667,23 +1671,27 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 		break;
 
 	case FILE_INDIRECT:
 		if (offset == 0)
 			return 0;
+
 		if (nbytes < offset)
 			return 0;
-		sbuf = ms->o.buf;
-		soffset = ms->offset;
-		ms->o.buf = NULL;
-		ms->offset = 0;
+
+		if ((pb = file_push_buffer(ms)) == NULL)
+			return -1;
+
 		rv = file_softmagic(ms, s + offset, nbytes - offset,
 		    recursion_level, BINTEST, text);
+
 		if ((ms->flags & MAGIC_DEBUG) != 0)
 			fprintf(stderr, "indirect @offs=%u[%d]\n", offset, rv);
-		rbuf = ms->o.buf;
-		ms->o.buf = sbuf;
-		ms->offset = soffset;
+
+		rbuf = file_pop_buffer(ms, pb);
+		if (rbuf == NULL)
+			return -1;
+
 		if (rv == 1) {
 			if ((ms->flags & (MAGIC_MIME|MAGIC_APPLE)) == 0 &&
 			    file_printf(ms, F(ms, m, "%u"), offset) == -1) {
 				free(rbuf);
 				return -1;
@@ -1697,17 +1705,17 @@ mget(struct magic_set *ms, const unsigned char *s, struct magic *m,
 		return rv;
 
 	case FILE_USE:
 		if (nbytes < offset)
 			return 0;
-		sbuf = m->value.s;
-		if (*sbuf == '^') {
-			sbuf++;
+		rbuf = m->value.s;
+		if (*rbuf == '^') {
+			rbuf++;
 			flip = !flip;
 		}
-		if (file_magicfind(ms, sbuf, &ml) == -1) {
-			file_error(ms, 0, "cannot find entry `%s'", sbuf);
+		if (file_magicfind(ms, rbuf, &ml) == -1) {
+			file_error(ms, 0, "cannot find entry `%s'", rbuf);
 			return -1;
 		}
 
 		oneed_separator = *need_separator;
 		if (m->flag & NOSPACE)

Vulnerable code:  * @(#)$File: file.h,v 1.154 2014/09/10 18:41:51 christos Exp $
FILE_RCSID("@(#)$File: funcs.c,v 1.72 2014/05/14 23:15:42 christos Exp $")
FILE_RCSID("@(#)$File: softmagic.c,v 1.196 2014/11/07 15:24:14 christos Exp $")
	uint32_t soffset, offset = ms->offset;
	char *sbuf, *rbuf;
	if (recursion_level >= 20) {
		sbuf = ms->o.buf;
		soffset = ms->offset;
		ms->o.buf = NULL;
		ms->offset = 0;
		rbuf = ms->o.buf;
		ms->o.buf = sbuf;
		ms->offset = soffset;
		sbuf = m->value.s;
		if (*sbuf == '^') {
			sbuf++;
		if (file_magicfind(ms, sbuf, &ml) == -1) {
			file_error(ms, 0, "cannot find entry `%s'", sbuf);
Fixed code:  * @(#)$File: file.h,v 1.155 2014/10/11 15:03:16 christos Exp $
typedef struct {
	char *buf;
	uint32_t offset;
} file_pushbuf_t;

protected file_pushbuf_t *file_push_buffer(struct magic_set *);
protected char  *file_pop_buffer(struct magic_set *, file_pushbuf_t *);

FILE_RCSID("@(#)$File: funcs.c,v 1.73 2014/09/10 18:41:51 christos Exp $")

protected file_pushbuf_t *
file_push_buffer(struct magic_set *ms)
{
	file_pushbuf_t *pb;

	if (ms->event_flags & EVENT_HAD_ERR)
		return NULL;

	if ((pb = (CAST(file_pushbuf_t *, malloc(sizeof(*pb))))) == NULL)
		return NULL;

	pb->buf = ms->o.buf;
	pb->offset = ms->offset;

	ms->o.buf = NULL;
	ms->offset = 0;

	return pb;
}

protected char *
file_pop_buffer(struct magic_set *ms, file_pushbuf_t *pb)
{
	char *rbuf;

	if (ms->event_flags & EVENT_HAD_ERR) {
		free(pb->buf);
		free(pb);
		return NULL;
	}

	rbuf = ms->o.buf;

	ms->o.buf = pb->buf;
	ms->offset = pb->offset;

	free(pb);
	return rbuf;
}
FILE_RCSID("@(#)$File: softmagic.c,v 1.197 2014/11/11 17:48:23 christos Exp $")

#define MAX_RECURSION_LEVEL	10

	uint32_t offset = ms->offset;
	file_pushbuf_t *pb;
	char *rbuf;
	if (recursion_level >= MAX_RECURSION_LEVEL) {


		if ((pb = file_push_buffer(ms)) == NULL)
			return -1;



		rbuf = file_pop_buffer(ms, pb);
		if (rbuf == NULL)
			return -1;

		rbuf = m->value.s;
		if (*rbuf == '^') {
			rbuf++;
		if (file_magicfind(ms, rbuf, &ml) == -1) {
			file_error(ms, 0, "cannot find entry `%s'", rbuf);