io: return error when reading/writing to unmapped memory (#3323)

* core: 'wow' command was replaced with 'wb'

* io: on_map_skyline returns false when nothing was done

Before this patch on_map_skyline returned true even when the callback
was not called on any part of the skyline. In other words,
reading/writing to an unmapped memory returned true.

This commit changes the behaviour to return false in those cases.

* analysis/esil: mem_read/mem_write were alway returning true

* use o malloc instead of oC

* librz: deal with seek errors

librz/analysis/esil/esil.c

@@ -314,10 +314,8 @@ static int internal_esil_mem_write_no_null(RzAnalysisEsil *esil, ut64 addr, cons
 }
 
 RZ_API int rz_analysis_esil_mem_write(RzAnalysisEsil *esil, ut64 addr, const ut8 *buf, int len) {
+	rz_return_val_if_fail(esil && buf, 0);
 	int ret = 0;
-	if (!buf || !esil) {
-		return 0;
-	}
 	addr &= esil->addrmask;
 	if (esil->cb.hook_mem_write) {
 		ret = esil->cb.hook_mem_write(esil, addr, buf, len);
@@ -1809,14 +1807,13 @@ static bool esil_poke_n(RzAnalysisEsil *esil, int bits) {
 				src2 = rz_analysis_esil_pop(esil);
 				if (src2 && rz_analysis_esil_get_parm(esil, src2, &num2)) {
 					rz_write_ble(b, num, esil->analysis->big_endian, 64);
-					ret = rz_analysis_esil_mem_write(esil, addr, b, bytes);
-					if (ret == 0) {
-						rz_write_ble(b, num2, esil->analysis->big_endian, 64);
-						ret = rz_analysis_esil_mem_write(esil, addr + 8, b, bytes);
-					}
+					rz_analysis_esil_mem_write(esil, addr, b, bytes);
+					rz_write_ble(b, num2, esil->analysis->big_endian, 64);
+					rz_analysis_esil_mem_write(esil, addr + 8, b, bytes);
+					ret = true;
 					goto out;
 				}
-				ret = -1;
+				ret = false;
 				goto out;
 			}
 			// this is a internal peek performed before a poke
@@ -1831,7 +1828,8 @@ static bool esil_poke_n(RzAnalysisEsil *esil, int bits) {
 			esil->lastsz = bits;
 			num = num & bitmask;
 			rz_write_ble(b, num, esil->analysis->big_endian, bits);
-			ret = rz_analysis_esil_mem_write(esil, addr, b, bytes);
+			rz_analysis_esil_mem_write(esil, addr, b, bytes);
+			ret = true;
 		}
 	}
 out:
@@ -1931,23 +1929,24 @@ static bool esil_peek_n(RzAnalysisEsil *esil, int bits) {
 	if (dst && isregornum(esil, dst, &addr)) {
 		if (bits == 128) {
 			ut8 a[sizeof(ut64) * 2] = { 0 };
-			ret = rz_analysis_esil_mem_read(esil, addr, a, bytes);
+			rz_analysis_esil_mem_read(esil, addr, a, bytes);
 			ut64 b = rz_read_ble(a, esil->analysis->big_endian, bits);
 			ut64 c = rz_read_ble(a + 8, esil->analysis->big_endian, bits);
 			rz_strf(res, "0x%" PFMT64x, b);
 			rz_analysis_esil_push(esil, res);
 			rz_strf(res, "0x%" PFMT64x, c);
 			rz_analysis_esil_push(esil, res);
 			free(dst);
-			return ret;
+			return true;
 		}
 		ut64 bitmask = genmask(bits - 1);
 		ut8 a[sizeof(ut64)] = { 0 };
-		ret = rz_analysis_esil_mem_read(esil, addr, a, bytes);
+		rz_analysis_esil_mem_read(esil, addr, a, bytes);
 		ut64 b = rz_read_ble(a, esil->analysis->big_endian, bits);
 		rz_strf(res, "0x%" PFMT64x, b & bitmask);
 		rz_analysis_esil_push(esil, res);
 		esil->lastsz = bits;
+		ret = true;
 	}
 	free(dst);
 	return ret;

librz/bin/format/mach0/dyldcache_rebase.c

@@ -260,7 +260,11 @@ static ut64 buf_get_size(RzBuffer *b) {
 
 static st64 buf_seek(RzBuffer *b, st64 addr, int whence) {
 	BufCtx *ctx = b->priv;
-	return ctx->off = rz_seek_offset(ctx->off, rz_buf_size(b), addr, whence);
+	st64 val = rz_seek_offset(ctx->off, rz_buf_size(b), addr, whence);
+	if (val == -1) {
+		return -1;
+	}
+	return ctx->off = (ut64)val;
 }
 
 static ut8 *buf_get_whole_buf(RzBuffer *b, ut64 *sz) {

librz/core/cil.c

@@ -101,7 +101,7 @@ static void initialize_stack(RzCore *core, ut64 addr, ut64 size) {
 				break;
 			case 'z': // "zero"
 			case '0':
-				rz_core_cmdf(core, "wow 00 @ 0x%" PFMT64x "!0x%" PFMT64x, addr + i, left);
+				rz_core_cmdf(core, "wb 00 @ 0x%" PFMT64x "!0x%" PFMT64x, addr + i, left);
 				break;
 			}
 		}

librz/core/cio.c

@@ -141,7 +141,6 @@ RZ_API void rz_core_seek_arch_bits(RzCore *core, ut64 addr) {
 	}
 }
 
-// TODO: kill this wrapper
 RZ_API bool rz_core_write_at(RzCore *core, ut64 addr, const ut8 *buf, int size) {
 	rz_return_val_if_fail(core && buf && addr != UT64_MAX, false);
 	if (size < 1) {

librz/core/cmd/cmd_resize.c

@@ -37,7 +37,7 @@ RZ_IPI RzCmdStatus rz_resize_handler(RzCore *core, int argc, const char **argv,
 	if (!core->file) {
 		return RZ_CMD_STATUS_ERROR;
 	}
-	ut64 oldsize = (core->file) ? rz_io_fd_size(core->io, core->file->fd) : 0;
+	ut64 oldsize = rz_io_fd_size(core->io, core->file->fd);
 	if (oldsize == -1) {
 		return RZ_CMD_STATUS_ERROR;
 	}

librz/core/cmd/cmd_write.c

@@ -427,7 +427,7 @@ RZ_IPI RzCmdStatus rz_write_random_handler(RzCore *core, int argc, const char **
 		return RZ_CMD_STATUS_ERROR;
 	}
 	size_t length = rz_num_math(core->num, argv[1]);
-	return rz_core_write_random_at(core, core->offset, length) ? RZ_CMD_STATUS_OK : RZ_CMD_STATUS_ERROR;
+	return bool2status(rz_core_write_random_at(core, core->offset, length));
 }
 
 RZ_IPI RzCmdStatus rz_write_handler(RzCore *core, int argc, const char **argv) {
@@ -443,7 +443,7 @@ RZ_IPI RzCmdStatus rz_write_wide_string_handler(RzCore *core, int argc, const ch
 }
 
 RZ_IPI RzCmdStatus rz_write_hex_handler(RzCore *core, int argc, const char **argv) {
-	return rz_core_write_hexpair(core, core->offset, argv[1]) > 0 ? RZ_CMD_STATUS_OK : RZ_CMD_STATUS_ERROR;
+	return bool2status(rz_core_write_hexpair(core, core->offset, argv[1]) > 0);
 }
 
 RZ_IPI RzCmdStatus rz_write_hex_from_file_handler(RzCore *core, int argc, const char **argv) {

librz/core/tui/panels.c

@@ -3207,7 +3207,7 @@ int __assemble_cb(void *user) {
 
 int __fill_cb(void *user) {
 	RzCore *core = (RzCore *)user;
-	__add_cmdf_panel(core, "Fill with: ", "wow %s");
+	__add_cmdf_panel(core, "Fill with: ", "wb %s");
 	return 0;
 }
 

librz/core/tui/visual.c

@@ -1825,9 +1825,13 @@ static bool insert_mode_enabled(RzCore *core) {
 	case 'f':
 		if (visual->insertNibble != -1) {
 			char hexpair[3] = { visual->insertNibble, ch, 0 };
-			rz_core_write_hexpair(core, core->offset + core->print->cur, hexpair);
-			core->print->cur++;
+			bool res = rz_core_write_hexpair(core, core->offset + core->print->cur, hexpair);
 			visual->insertNibble = -1;
+			if (!res) {
+				RZ_LOG_ERROR("Cannot write at address %" PFMT64x ".\n", core->offset + core->print->cur);
+				break;
+			}
+			core->print->cur++;
 		} else {
 			visual->insertNibble = ch;
 		}
@@ -2449,7 +2453,7 @@ RZ_IPI int rz_core_visual_cmd(RzCore *core, const char *arg) {
 			rz_cons_flush();
 			rz_cons_set_raw(0);
 			if (ch == 'I') {
-				strcpy(buf, "wow ");
+				strcpy(buf, "wb ");
 				rz_line_set_prompt("insert hexpair block: ");
 				if (rz_cons_fgets(buf + 4, sizeof(buf) - 4, 0, NULL) < 0) {
 					buf[0] = '\0';
@@ -2477,7 +2481,7 @@ RZ_IPI int rz_core_visual_cmd(RzCore *core, const char *arg) {
 				if (core->print->ocur != -1) {
 					int bs = RZ_ABS(core->print->cur - core->print->ocur) + 1;
 					core->blocksize = bs;
-					strcpy(buf, "wow ");
+					strcpy(buf, "wb ");
 				} else {
 					strcpy(buf, "wx ");
 				}

librz/include/rz_util/rz_buf.h

@@ -64,17 +64,26 @@ typedef enum {
 /* utils */
 
 /// change cur according to addr and whence (RZ_BUF_SET/RZ_BUF_CUR/RZ_BUF_END)
-static inline ut64 rz_seek_offset(ut64 cur, ut64 length, st64 addr, int whence) {
+static inline st64 rz_seek_offset(ut64 cur, ut64 length, st64 addr, int whence) {
 	switch (whence) {
 	case RZ_BUF_CUR:
-		return cur + (ut64)addr;
+		if (ST64_ADD_OVFCHK((st64)cur, addr)) {
+			return -1;
+		}
+		return cur + addr;
 	case RZ_BUF_SET:
+		if ((st64)addr < 0) {
+			return -1;
+		}
 		return addr;
 	case RZ_BUF_END:
-		return length + (ut64)addr;
+		if (ST64_ADD_OVFCHK((st64)length, addr)) {
+			return -1;
+		}
+		return length + addr;
 	default:
 		rz_warn_if_reached();
-		return cur;
+		return -1;
 	}
 }
 

librz/io/io.c

@@ -31,7 +31,7 @@ static st64 on_map_skyline(RzIO *io, ut64 vaddr, ut8 *buf, int len, int match_fl
 	RzVector *skyline = &io->map_skyline.v;
 	ut64 addr = vaddr;
 	size_t i;
-	bool ret = true, wrap = !prefix_mode && vaddr + len < vaddr;
+	bool ret = false, wrap = !prefix_mode && vaddr + len < vaddr;
 #define CMP(addr, part) ((addr) < rz_itv_end(((RzSkylineItem *)(part))->itv) - 1 ? -1 : (addr) > rz_itv_end(((RzSkylineItem *)(part))->itv) - 1 ? 1 \
 																		: 0)
 	// Let i be the first skyline part whose right endpoint > addr
@@ -49,7 +49,7 @@ static st64 on_map_skyline(RzIO *io, ut64 vaddr, ut8 *buf, int len, int match_fl
 	while (i < rz_vector_len(skyline)) {
 		const RzSkylineItem *part = rz_vector_index_ptr(skyline, i);
 		// Right endpoint <= addr
-		if (rz_itv_end(part->itv) - 1 < addr) {
+		if (rz_itv_end(part->itv) == 0 || rz_itv_end(part->itv) - 1 < addr) {
 			i++;
 			if (wrap && i == rz_vector_len(skyline)) {
 				wrap = false;
@@ -84,9 +84,7 @@ static st64 on_map_skyline(RzIO *io, ut64 vaddr, ut8 *buf, int len, int match_fl
 					break;
 				}
 			} else {
-				if (result != len1) {
-					ret = false;
-				}
+				ret = result == len1;
 				addr += len1;
 			}
 		} else if (prefix_mode) {
@@ -306,7 +304,7 @@ RZ_API bool rz_io_read_at(RzIO *io, ut64 addr, ut8 *buf, int len) {
 		? rz_io_vread_at_mapped(io, addr, buf, len)
 		: rz_io_pread_at(io, addr, buf, len) > 0;
 	if (io->cached & RZ_PERM_R) {
-		(void)rz_io_cache_read(io, addr, buf, len);
+		ret |= rz_io_cache_read(io, addr, buf, len);
 	}
 	return ret;
 }
@@ -327,7 +325,7 @@ RZ_API bool rz_io_read_at_mapped(RzIO *io, ut64 addr, ut8 *buf, int len) {
 		ret = rz_io_pread_at(io, addr, buf, len) > 0;
 	}
 	if (io->cached & RZ_PERM_R) {
-		(void)rz_io_cache_read(io, addr, buf, len);
+		ret |= rz_io_cache_read(io, addr, buf, len);
 	}
 	return ret;
 }

librz/io/io_desc.c

@@ -349,14 +349,16 @@ RZ_API bool rz_io_desc_get_base(RzIODesc *desc, ut64 *base) {
 }
 
 RZ_API int rz_io_desc_read_at(RzIODesc *desc, ut64 addr, ut8 *buf, int len) {
-	if (desc && buf && (rz_io_desc_seek(desc, addr, RZ_IO_SEEK_SET) == addr)) {
+	ut64 val = rz_io_desc_seek(desc, addr, RZ_IO_SEEK_SET);
+	if (desc && buf && val != UT64_MAX && val == addr) {
 		return rz_io_desc_read(desc, buf, len);
 	}
 	return 0;
 }
 
 RZ_API int rz_io_desc_write_at(RzIODesc *desc, ut64 addr, const ut8 *buf, int len) {
-	if (desc && buf && (rz_io_desc_seek(desc, addr, RZ_IO_SEEK_SET) == addr)) {
+	ut64 val = rz_io_desc_seek(desc, addr, RZ_IO_SEEK_SET);
+	if (desc && buf && val != UT64_MAX && val == addr) {
 		return rz_io_desc_write(desc, buf, len);
 	}
 	return 0;

librz/io/p/io_default.c

@@ -34,8 +34,11 @@ static ut64 rz_io_def_mmap_seek(RzIO *io, RzIOMMapFileObj *mmo, ut64 offset, int
 	// NOTE: io->off should not be set here and code outside RzIO should not
 	// rely on it to get the current seek. They should use `rz_io_seek` instead.
 	// Keep it here until all use cases of io->off are converted.
-	io->off = rz_buf_seek(mmo->buf, offset, iowhence2buf(whence));
-	return io->off;
+	st64 val = rz_buf_seek(mmo->buf, offset, iowhence2buf(whence));
+	if (val == -1) {
+		return -1;
+	}
+	return io->off = val;
 }
 
 static void rz_io_def_mmap_free(RzIOMMapFileObj *mmo) {

librz/util/buf_bytes.c

@@ -101,10 +101,11 @@ static ut64 buf_bytes_get_size(RzBuffer *b) {
 
 static st64 buf_bytes_seek(RzBuffer *b, st64 addr, int whence) {
 	struct buf_bytes_priv *priv = get_priv_bytes(b);
-	if (addr < 0 && (-addr) > (st64)priv->offset) {
+	st64 val = rz_seek_offset(priv->offset, priv->length, addr, whence);
+	if (val == -1) {
 		return -1;
 	}
-	return priv->offset = rz_seek_offset(priv->offset, priv->length, addr, whence);
+	return priv->offset = val;
 }
 
 static ut8 *buf_bytes_get_whole_buf(RzBuffer *b, ut64 *sz) {

librz/util/buf_io.c

@@ -31,7 +31,11 @@ static bool buf_io_fini(RzBuffer *b) {
 
 static st64 buf_io_seek(RzBuffer *b, st64 addr, int whence) {
 	BufIOPriv *priv = b->priv;
-	priv->offset = rz_seek_offset(priv->offset, 0, addr, whence);
+	st64 val = rz_seek_offset(priv->offset, 0, addr, whence);
+	if (val == -1) {
+		return -1;
+	}
+	priv->offset = val;
 	// can't express the seek right if the highest bit is set,
 	// but at least tell the caller there was no error:
 	return RZ_MIN(priv->offset, ST64_MAX);

test/db/cmd/cmd_open

@@ -319,12 +319,12 @@ EXPECT=<<EOF
 EOF
 RUN
 
-NAME=oC ob fix
+NAME=malloc ob fix
 FILE=--
 CMDS=<<EOF
 e asm.arch=x86
 e asm.bits=64
-oC 1024
+o malloc://1024
 obl
 EOF
 EXPECT=<<EOF

test/db/cmd/cmd_pde

@@ -63,8 +63,6 @@ EXPECT=<<EOF
 0x0001018c ldr lr, [pc, 4]
 0x00010190 add lr, pc, lr
 0x00010194 ldr pc, [lr, 8]!
-0x00000000   cpsr:
-0x00000000 invalid
 EOF
 RUN
 

test/unit/test_core_bin.c

@@ -172,7 +172,7 @@ bool test_map(void) {
 
 	ut8 buf[8];
 	r = rz_io_read_at(core->io, 0, buf, sizeof(buf));
-	mu_assert_true(r, "io read");
+	mu_assert_false(r, "io read");
 	mu_assert_memeq(buf, (const ut8 *)"\xff\xff\xff\xff\xff\xff\xff\xff", 8, "unmapped read");
 	r = rz_io_read_at(core->io, 0xfe, buf, sizeof(buf));
 	mu_assert_true(r, "io read");
@@ -253,10 +253,10 @@ bool test_cfile_close(void) {
 
 	ut8 buf[8];
 	r = rz_io_read_at(core->io, 0xfe, buf, sizeof(buf));
-	mu_assert_true(r, "io read");
+	mu_assert_false(r, "io read");
 	mu_assert_memeq(buf, (const ut8 *)"\xff\xff\xff\xff\xff\xff\xff\xff", 8, "direct map read after close");
 	r = rz_io_read_at(core->io, 0x22e, buf, sizeof(buf));
-	mu_assert_true(r, "io read");
+	mu_assert_false(r, "io read");
 	mu_assert_memeq(buf, (const ut8 *)"\xff\xff\xff\xff\xff\xff\xff\xff", 8, "direct map read with zeroes end after close");
 
 	rz_core_free(core);
@@ -498,7 +498,7 @@ bool test_cfile_close_manual_vfile_fd(void) {
 	mu_assert_eq(rz_pvector_len(&core->io->maps), 0, "io maps count");
 
 	r = rz_io_read_at(core->io, 0x8000, buf, sizeof(buf));
-	mu_assert_true(r, "io read");
+	mu_assert_false(r, "io read");
 	mu_assert_memeq(buf, (const ut8 *)"\xff\xff\xff\xff\xff\xff\xff\xff", 8, "manual vfile map read");
 
 	rz_core_free(core);
@@ -538,7 +538,7 @@ bool test_cfile_close_manual_vfile_map(void) {
 	// now everything should be gone, including the vfile map which indirectly pointed into the core file's vfile fd
 	mu_assert_eq(rz_pvector_len(&core->io->maps), 0, "io maps count");
 	r = rz_io_read_at(core->io, 0x8000, buf, sizeof(buf));
-	mu_assert_true(r, "io read");
+	mu_assert_false(r, "io read");
 	mu_assert_memeq(buf, (const ut8 *)"\xff\xff\xff\xff\xff\xff\xff\xff", 8, "manual vfile map read");
 
 	rz_core_free(core);