CVE-2023-27590: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: Rizin is a UNIX-like reverse engineering framework and command-line toolset. In version 0.5.1 and prior, converting a GDB registers profile file into a Rizin register profile can result in a stack-based buffer overflow when the `name`, `type`, or `groups` fields have longer values than expected. Users opening untrusted GDB registers files (e.g. with the `drpg` or `arpg` commands) are affected by this flaw. Commit d6196703d89c84467b600ba2692534579dc25ed4 contains a patch for this issue. As a workaround, review the GDB register profiles before loading them with `drpg`/`arpg` commands.
CWE description: The product copies an input buffer to an output buffer without verifying that the size of the input buffer is less than the size of the output buffer, leading to a buffer overflow.
CVSS v3 base score: 7.8
Published: 2023-03-14T21:15Z
Repository: https://github.com/rizinorg/rizin
Language: C
Files changed: librz/debug/p/debug_gdb.c, librz/debug/p/debug_io.c, librz/include/rz_types_base.h, librz/reg/profile.c
Commit date: 2023-03-14 11:41:29 +0100
Commit message: Fix conversion from GDB register profile to rizin profile

Diff: diff --git a/librz/debug/p/debug_gdb.c b/librz/debug/p/debug_gdb.c
index 6778d1968f..ece159a6f6 100644
--- a/librz/debug/p/debug_gdb.c
+++ b/librz/debug/p/debug_gdb.c
@@ -13,10 +13,17 @@ typedef struct {
 
 #define UNKNOWN     (-1)
 #define UNSUPPORTED 0
 #define SUPPORTED   1
 
+#define PROC_NAME_SZ   1024
+#define PROC_REGION_SZ 100
+// PROC_REGION_SZ - 2 (used for `0x`). Due to how RZ_STR_DEF works this can't be
+// computed.
+#define PROC_REGION_LEFT_SZ 98
+#define PROC_PERM_SZ        5
+
 typedef struct rz_debug_gdb_ctx_t {
 	RzIOGdb **origrziogdb;
 	libgdbr_t *desc;
 	ut8 *reg_buf;
 	int buf_size;
@@ -187,11 +194,11 @@ static RzList /*<RzDebugMap *>*/ *rz_debug_gdb_map_get(RzDebug *dbg) { // TODO
 
 	// Get map list
 	int unk = 0, perm, i;
 	char *ptr, *pos_1;
 	size_t line_len;
-	char name[1024], region1[100], region2[100], perms[5];
+	char name[PROC_NAME_SZ + 1], region1[PROC_REGION_SZ + 1], region2[PROC_REGION_SZ + 1], perms[PROC_PERM_SZ + 1];
 	RzDebugMap *map = NULL;
 	region1[0] = region2[0] = '0';
 	region1[1] = region2[1] = 'x';
 	if (!(ptr = strtok((char *)buf, "\n"))) {
 		gdbr_close_file(ctx->desc);
@@ -211,12 +218,12 @@ static RzList /*<RzDebugMap *>*/ *rz_debug_gdb_map_get(RzDebug *dbg) { // TODO
 		if (line_len == 0) {
 			break;
 		}
 		// We assume Linux target, for now, so -
 		// 7ffff7dda000-7ffff7dfd000 r-xp 00000000 08:05 265428 /usr/lib/ld-2.25.so
-		ret = sscanf(ptr, "%s %s %" PFMT64x " %*s %*s %[^\n]", &region1[2],
-			perms, &offset, name);
+		ret = sscanf(ptr, "%" RZ_STR_DEF(PROC_REGION_LEFT_SZ) "s %" RZ_STR_DEF(PROC_PERM_SZ) "s %" PFMT64x " %*s %*s %" RZ_STR_DEF(PROC_NAME_SZ) "[^\n]",
+			&region1[2], perms, &offset, name);
 		if (ret == 3) {
 			name[0] = '\0';
 		} else if (ret != 4) {
 			eprintf("%s: Unable to parse \"%s\"\nContent:\n%s\n",
 				__func__, path, buf);
diff --git a/librz/debug/p/debug_io.c b/librz/debug/p/debug_io.c
index 940aeae8ee..ea79ce567f 100644
--- a/librz/debug/p/debug_io.c
+++ b/librz/debug/p/debug_io.c
@@ -2,10 +2,13 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 #include <rz_io.h>
 #include <rz_debug.h>
 
+#define IO_MAPS_PERM_SZ 32
+#define IO_MAPS_NAME_SZ 512
+
 static int __io_step(RzDebug *dbg) {
 	free(dbg->iob.system(dbg->iob.io, "ds"));
 	return true;
 }
 
@@ -21,12 +24,12 @@ static RzList /*<RzDebugMap *>*/ *__io_maps(RzDebug *dbg) {
 		rz_list_free(list);
 		return NULL;
 	}
 	char *ostr = str;
 	ut64 map_start, map_end;
-	char perm[32];
-	char name[512];
+	char perm[IO_MAPS_PERM_SZ + 1];
+	char name[IO_MAPS_NAME_SZ + 1];
 	for (;;) {
 		char *nl = strchr(str, '\n');
 		if (nl) {
 			*nl = 0;
 			*name = 0;
@@ -46,11 +49,11 @@ static RzList /*<RzDebugMap *>*/ *__io_maps(RzDebug *dbg) {
 			}
 			_s_ = strstr(str, " ? ");
 			if (_s_) {
 				memmove(_s_, _s_ + 2, strlen(_s_));
 			}
-			sscanf(str, "0x%" PFMT64x " - 0x%" PFMT64x " %s %s",
+			sscanf(str, "0x%" PFMT64x " - 0x%" PFMT64x " %" RZ_STR_DEF(IO_MAPS_PERM_SZ) "s %" RZ_STR_DEF(IO_MAPS_NAME_SZ) "s",
 				&map_start, &map_end, perm, name);
 			if (map_end != 0LL) {
 				RzDebugMap *map = rz_debug_map_new(name, map_start, map_end, rz_str_rwx(perm), 0);
 				rz_list_append(list, map);
 			}
diff --git a/librz/include/rz_types_base.h b/librz/include/rz_types_base.h
index ab154958d5..94a038cf31 100644
--- a/librz/include/rz_types_base.h
+++ b/librz/include/rz_types_base.h
@@ -212,6 +212,9 @@ typedef struct _utX {
 	static x *x##_new(x n) { \
 		x *m = malloc(sizeof(x)); \
 		return m ? *m = n, m : m; \
 	}
 
+#define RZ_STR_DEF(s) RZ_STR(s)
+#define RZ_STR(s)     #s
+
 #endif // RZ_TYPES_BASE_H
diff --git a/librz/reg/profile.c b/librz/reg/profile.c
index b0b882828f..0be46fd98e 100644
--- a/librz/reg/profile.c
+++ b/librz/reg/profile.c
@@ -8,10 +8,14 @@
 #include <rz_util.h>
 #include <rz_util/rz_assert.h>
 #include <rz_lib.h>
 #include <string.h>
 
+#define GDB_NAME_SZ   16
+#define GDB_TYPE_SZ   16
+#define GDB_GROUPS_SZ 128
+
 static void rz_reg_profile_def_free(RzRegProfileDef *def) {
 	if (!def) {
 		return;
 	}
 	free(def->name);
@@ -509,11 +513,11 @@ static char *gdb_to_rz_profile(const char *gdb) {
 	RzStrBuf *sb = rz_strbuf_new("");
 	if (!sb) {
 		return NULL;
 	}
 	char *ptr1, *gptr, *gptr1;
-	char name[16], groups[128], type[16];
+	char name[GDB_NAME_SZ + 1], groups[GDB_GROUPS_SZ + 1], type[GDB_TYPE_SZ + 1];
 	const int all = 1, gpr = 2, save = 4, restore = 8, float_ = 16,
 		  sse = 32, vector = 64, system = 128, mmx = 256;
 	int number, rel, offset, size, type_bits, ret;
 	// Every line is -
 	// Name Number Rel Offset Size Type Groups
@@ -540,12 +544,12 @@ static char *gdb_to_rz_profile(const char *gdb) {
 		} else {
 			eprintf("Could not parse line: %s (missing \\n)\n", ptr);
 			rz_strbuf_free(sb);
 			return false;
 		}
-		ret = sscanf(ptr, " %s %d %d %d %d %s %s", name, &number, &rel,
-			&offset, &size, type, groups);
+		ret = sscanf(ptr, " %" RZ_STR_DEF(GDB_NAME_SZ) "s %d %d %d %d %" RZ_STR_DEF(GDB_TYPE_SZ) "s %" RZ_STR_DEF(GDB_GROUPS_SZ) "s",
+			name, &number, &rel, &offset, &size, type, groups);
 		// Groups is optional, others not
 		if (ret < 6) {
 			if (*ptr != '*') {
 				eprintf("Could not parse line: %s\n", ptr);
 				rz_strbuf_free(sb);

Vulnerable code: 	char name[1024], region1[100], region2[100], perms[5];
		ret = sscanf(ptr, "%s %s %" PFMT64x " %*s %*s %[^\n]", &region1[2],
			perms, &offset, name);
	char perm[32];
	char name[512];
			sscanf(str, "0x%" PFMT64x " - 0x%" PFMT64x " %s %s",
	char name[16], groups[128], type[16];
		ret = sscanf(ptr, " %s %d %d %d %d %s %s", name, &number, &rel,
			&offset, &size, type, groups);
Fixed code: #define PROC_NAME_SZ   1024
#define PROC_REGION_SZ 100
// PROC_REGION_SZ - 2 (used for `0x`). Due to how RZ_STR_DEF works this can't be
// computed.
#define PROC_REGION_LEFT_SZ 98
#define PROC_PERM_SZ        5

	char name[PROC_NAME_SZ + 1], region1[PROC_REGION_SZ + 1], region2[PROC_REGION_SZ + 1], perms[PROC_PERM_SZ + 1];
		ret = sscanf(ptr, "%" RZ_STR_DEF(PROC_REGION_LEFT_SZ) "s %" RZ_STR_DEF(PROC_PERM_SZ) "s %" PFMT64x " %*s %*s %" RZ_STR_DEF(PROC_NAME_SZ) "[^\n]",
			&region1[2], perms, &offset, name);
#define IO_MAPS_PERM_SZ 32
#define IO_MAPS_NAME_SZ 512

	char perm[IO_MAPS_PERM_SZ + 1];
	char name[IO_MAPS_NAME_SZ + 1];
			sscanf(str, "0x%" PFMT64x " - 0x%" PFMT64x " %" RZ_STR_DEF(IO_MAPS_PERM_SZ) "s %" RZ_STR_DEF(IO_MAPS_NAME_SZ) "s",
#define RZ_STR_DEF(s) RZ_STR(s)
#define RZ_STR(s)     #s

#define GDB_NAME_SZ   16
#define GDB_TYPE_SZ   16
#define GDB_GROUPS_SZ 128

	char name[GDB_NAME_SZ + 1], groups[GDB_GROUPS_SZ + 1], type[GDB_TYPE_SZ + 1];
		ret = sscanf(ptr, " %" RZ_STR_DEF(GDB_NAME_SZ) "s %d %d %d %d %" RZ_STR_DEF(GDB_TYPE_SZ) "s %" RZ_STR_DEF(GDB_GROUPS_SZ) "s",
			name, &number, &rel, &offset, &size, type, groups);