Fix #14303 - oob crash in RParse api usage, needs API redesign

radareorg · Jun 15, 2019 · eef6fdd · eef6fdd
1 parent 4e668db
commit eef6fdd
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 10 deletions.
diff --git a/libr/asm/asm.c b/libr/asm/asm.c
@@ -19,6 +19,20 @@ static char *directives[] = {
 
 static RAsmPlugin *asm_static_plugins[] = { R_ASM_STATIC_PLUGINS };
 
+static void parseHeap(RParse *p, RStrBuf *s) {
+	char *op_buf_asm = r_strbuf_get (s);
+	size_t len = r_strbuf_length (s);
+	char *out = malloc (64 + (len * 2));
+	if (out) {
+		*out = 0;
+		strcpy (out , op_buf_asm);
+	// XXX we shouldnt pad here because we have t orefactor the RParse API to handle boundaries and chunks properly
+		r_parse_parse (p, op_buf_asm, out);
+		r_strbuf_set (s, out);
+		free (out);
+	}
+}
+
 /* pseudo.c - private api */
 static int r_asm_pseudo_align(RAsmCode *acode, RAsmOp *op, char *input) {
 	acode->code_align = r_num_math (NULL, input);
@@ -456,8 +470,7 @@ R_API int r_asm_disassemble(RAsm *a, RAsmOp *op, const ut8 *buf, int len) {
 		}
 	}
 	if (a->ofilter) {
-		char *buf_asm = r_strbuf_get (&op->buf_asm);
-		r_parse_parse (a->ofilter, buf_asm, buf_asm);
+		parseHeap (a->ofilter, &op->buf_asm);
 	}
 	int opsz = (op->size > 0)? R_MAX (0, R_MIN (len, op->size)): 1;
 	r_asm_op_set_buf (op, buf, opsz);
@@ -620,8 +633,7 @@ R_API RAsmCode* r_asm_mdisassemble(RAsm *a, const ut8 *buf, int len) {
 			ret = 1;
 		}
 		if (a->ofilter) {
-			char *op_buf_asm = r_strbuf_get (&op.buf_asm);
-			r_parse_parse (a->ofilter, op_buf_asm, op_buf_asm);
+			parseHeap (a->ofilter, &op.buf_asm);
 		}
 		r_strbuf_append (buf_asm, r_strbuf_get (&op.buf_asm));
 		r_strbuf_append (buf_asm, "\n");
@@ -643,6 +655,7 @@ R_API RAsmCode* r_asm_mdisassemble_hexstr(RAsm *a, RParse *p, const char *hexstr
 	}
 	RAsmCode *ret = r_asm_mdisassemble (a, buf, (ut64)len);
 	if (ret && p) {
+		// XXX this can crash
 		r_parse_parse (p, ret->assembly, ret->assembly);
 	}
 	free (buf);

diff --git a/libr/include/r_parse.h b/libr/include/r_parse.h
@@ -59,7 +59,7 @@ R_API void r_parse_set_user_ptr(RParse *p, void *user);
 R_API int r_parse_add(RParse *p, RParsePlugin *foo);
 R_API int r_parse_list(RParse *p);
 R_API int r_parse_use(RParse *p, const char *name);
-R_API int r_parse_parse(RParse *p, const char *data, char *str);
+R_API bool r_parse_parse(RParse *p, const char *data, char *str);
 R_API int r_parse_assemble(RParse *p, char *data, char *str);
 R_API int r_parse_filter(RParse *p, ut64 addr, RFlag *f, RAnalHint *hint, char *data, char *str, int len, bool big_endian);
 R_API bool r_parse_varsub(RParse *p, RAnalFunction *f, ut64 addr, int oplen, char *data, char *str, int len);

diff --git a/libr/parse/p/parse_avr_pseudo.c b/libr/parse/p/parse_avr_pseudo.c
@@ -1,4 +1,4 @@
-/* radare - LGPL - Copyright 2017 - wargio */
+/* radare - LGPL - Copyright 2017-2019 - wargio */
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -10,7 +10,7 @@
 #include <r_anal.h>
 #include <r_parse.h>
 
-static int replace(int argc, const char *argv[], char *newstr) {
+static bool replace(int argc, const char *argv[], char *newstr) {
 	int i,j,k;
 	struct {
 		char *op;
@@ -209,7 +209,7 @@ static int parse(RParse *p, const char *data, char *str) {
 					nw++;
 				}
 			}
-			replace (nw, wa, str);
+			(void)replace (nw, wa, str);
 		}
 	}
 	free (buf);

diff --git a/libr/parse/parse.c b/libr/parse/parse.c
@@ -91,9 +91,10 @@ R_API int r_parse_assemble(RParse *p, char *data, char *str) {
 	return ret;
 }
 
-// parse 'data' and generate pseudocode disassemble in 'str'
+// data is input disasm, str is output pseudo
 // TODO: refactooring, this should return char * instead
-R_API int r_parse_parse(RParse *p, const char *data, char *str) {
+R_API bool r_parse_parse(RParse *p, const char *data, char *str) {
+	r_return_val_if_fail (p && data && str, false);
 	if (p && data && *data && p->cur && p->cur->parse) {
 		return p->cur->parse (p, data, str);
 	}