CVE-2018-20458: Out-of-bounds Read (CWE-125)
value: In radare2 prior to 3.1.1, r_bin_dyldcache_extract in libr/bin/format/mach0/dyldcache.c may allow attackers to cause a denial-of-service (application crash caused by out-of-bounds read) by crafting an input file.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2018-12-25T19:29Z
Repository: https://github.com/radareorg/radare2
Language: C
Files changed: libr/bin/format/mach0/dyldcache.c
Commit date: 2018-12-03 02:03:00 +0100
Commit message: Fix #12374 - oobread crash in truncated dyldcache ##bin (#12381)


Diff: diff --git a/libr/bin/format/mach0/dyldcache.c b/libr/bin/format/mach0/dyldcache.c
index b19dbc656a..1fc7cc2b82 100644
--- a/libr/bin/format/mach0/dyldcache.c
+++ b/libr/bin/format/mach0/dyldcache.c
@@ -1,6 +1,6 @@
-/* radare - LGPL - Copyright 2010-2016 - nibble, pancake  */
+/* radare - LGPL - Copyright 2010-2018 - nibble, pancake  */
 
 #include <stdio.h>
 #include <r_types.h>
 #include <r_util.h>
 #include "dyldcache.h"
@@ -19,10 +19,27 @@ static int r_bin_dyldcache_apply_patch (struct r_buf_t* buf, ut32 data, ut64 off
 	return r_buf_write_at (buf, offset, (ut8*)&data, sizeof (data));
 }
 
 #define NZ_OFFSET(x) if((x) > 0) r_bin_dyldcache_apply_patch (dbuf, (x) - linkedit_offset, (ut64)((size_t)&(x) - (size_t)data))
 
+// make it public in util/buf.c ?
+static ut64 r_buf_read64le (RBuffer *buf, ut64 off) {
+	ut8 data[8] = {0};
+	r_buf_read_at (buf, off, data, 8);
+	return r_read_le64 (data);
+}
+
+static char *r_buf_read_string (RBuffer *buf, ut64 addr, int len) {
+	ut8 *data = malloc (len);
+	if (data) {
+		r_buf_read_at (buf, addr, data, len);
+		data[len-1] = 0;
+		return data;
+	}
+	return NULL;
+}
+
 /* TODO: Needs more testing and ERROR HANDLING */
 struct r_bin_dyldcache_lib_t *r_bin_dyldcache_extract(struct r_bin_dyldcache_obj_t* bin, int idx, int *nlib) {
 	ut64 liboff, linkedit_offset;
 	ut64 dyld_vmbase;
 	ut32 addend = 0;
@@ -45,38 +62,45 @@ struct r_bin_dyldcache_lib_t *r_bin_dyldcache_extract(struct r_bin_dyldcache_obj
 		return NULL;
 	}
 	*nlib = bin->nlibs;
 	ret = R_NEW0 (struct r_bin_dyldcache_lib_t);
 	if (!ret) {
-		perror ("malloc (ret)");
 		return NULL;
 	}
 	if (bin->hdr.startaddr > bin->size) {
 	    	eprintf ("corrupted dyldcache");
 		free (ret);
 		return NULL;
 	}
+
 	if (bin->hdr.startaddr > bin->size || bin->hdr.baseaddroff > bin->size) {
 		eprintf ("corrupted dyldcache");
 		free (ret);
 		return NULL;
 	}
-	image_infos = (struct dyld_cache_image_info*) (bin->b->buf + bin->hdr.startaddr);
-	dyld_vmbase = *(ut64 *)(bin->b->buf + bin->hdr.baseaddroff);
+	int sz = bin->nlibs * sizeof (struct dyld_cache_image_info);
+	image_infos = malloc (sz); //(struct dyld_cache_image_info*) (bin->b->buf + bin->hdr.startaddr);
+	if (!image_infos) {
+		free (ret);
+		return NULL;
+	}
+	r_buf_read_at (bin->b, bin->hdr.startaddr, (ut8*)image_infos, sz);
+	dyld_vmbase = r_buf_read64le (bin->b, bin->hdr.baseaddroff);
 	liboff = image_infos[idx].address - dyld_vmbase;
 	if (liboff > bin->size) {
 		eprintf ("Corrupted file\n");
 		free (ret);
 		return NULL;
 	}
 	ret->offset = liboff;
-	if (image_infos[idx].pathFileOffset > bin->size) {
-	    eprintf ("corrupted file\n");
+	int pfo = image_infos[idx].pathFileOffset;
+	if (pfo < 0 || pfo > bin->size) {
+		eprintf ("corrupted file: pathFileOffset > bin->size (%d)\n", pfo);
 		free (ret);
 		return NULL;
 	}
-	libname = (char *)(bin->b->buf + image_infos[idx].pathFileOffset);
+	libname = r_buf_read_string (bin->b, pfo, 64);
 	/* Locate lib hdr in cache */
 	data = bin->b->buf + liboff;
 	mh = (struct mach_header *)data;
 	/* Check it is mach-o */
 	if (mh->magic != MH_MAGIC && mh->magic != MH_MAGIC_64) {
@@ -222,20 +246,19 @@ struct r_bin_dyldcache_obj_t* r_bin_dyldcache_new(const char* file) {
 	}
 	return bin;
 }
 
 struct r_bin_dyldcache_obj_t* r_bin_dyldcache_from_bytes_new(const ut8* buf, ut64 size) {
-	struct r_bin_dyldcache_obj_t *bin;
-	if (!(bin = malloc (sizeof (struct r_bin_dyldcache_obj_t)))) {
+	struct r_bin_dyldcache_obj_t *bin = R_NEW0 (struct r_bin_dyldcache_obj_t);
+	if (!bin) {
 		return NULL;
 	}
-	memset (bin, 0, sizeof (struct r_bin_dyldcache_obj_t));
 	if (!buf) {
 		return r_bin_dyldcache_free (bin);
 	}
-	bin->b = r_buf_new();
-	if (!r_buf_set_bytes (bin->b, buf, size)) {
+	bin->b = r_buf_new ();
+	if (!bin->b || !r_buf_set_bytes (bin->b, buf, size)) {
 		return r_bin_dyldcache_free (bin);
 	}
 	if (!r_bin_dyldcache_init (bin)) {
 		return r_bin_dyldcache_free (bin);
 	}

Vulnerable code: /* radare - LGPL - Copyright 2010-2016 - nibble, pancake  */
		perror ("malloc (ret)");
	image_infos = (struct dyld_cache_image_info*) (bin->b->buf + bin->hdr.startaddr);
	dyld_vmbase = *(ut64 *)(bin->b->buf + bin->hdr.baseaddroff);
	if (image_infos[idx].pathFileOffset > bin->size) {
	    eprintf ("corrupted file\n");
	libname = (char *)(bin->b->buf + image_infos[idx].pathFileOffset);
	struct r_bin_dyldcache_obj_t *bin;
	if (!(bin = malloc (sizeof (struct r_bin_dyldcache_obj_t)))) {
	memset (bin, 0, sizeof (struct r_bin_dyldcache_obj_t));
	bin->b = r_buf_new();
	if (!r_buf_set_bytes (bin->b, buf, size)) {
Fixed code: /* radare - LGPL - Copyright 2010-2018 - nibble, pancake  */
// make it public in util/buf.c ?
static ut64 r_buf_read64le (RBuffer *buf, ut64 off) {
	ut8 data[8] = {0};
	r_buf_read_at (buf, off, data, 8);
	return r_read_le64 (data);
}

static char *r_buf_read_string (RBuffer *buf, ut64 addr, int len) {
	ut8 *data = malloc (len);
	if (data) {
		r_buf_read_at (buf, addr, data, len);
		data[len-1] = 0;
		return data;
	}
	return NULL;
}


	int sz = bin->nlibs * sizeof (struct dyld_cache_image_info);
	image_infos = malloc (sz); //(struct dyld_cache_image_info*) (bin->b->buf + bin->hdr.startaddr);
	if (!image_infos) {
		free (ret);
		return NULL;
	}
	r_buf_read_at (bin->b, bin->hdr.startaddr, (ut8*)image_infos, sz);
	dyld_vmbase = r_buf_read64le (bin->b, bin->hdr.baseaddroff);
	int pfo = image_infos[idx].pathFileOffset;
	if (pfo < 0 || pfo > bin->size) {
		eprintf ("corrupted file: pathFileOffset > bin->size (%d)\n", pfo);
	libname = r_buf_read_string (bin->b, pfo, 64);
	struct r_bin_dyldcache_obj_t *bin = R_NEW0 (struct r_bin_dyldcache_obj_t);
	if (!bin) {
	bin->b = r_buf_new ();
	if (!bin->b || !r_buf_set_bytes (bin->b, buf, size)) {