[PATCH] box: fix use-after-free in space_truncate

[PATCH] box: fix use-after-free in space_truncate

[Vladimir Davydov]

space_truncate allocates a statement on the stack which is grossly
incorrect as the stack may be purged once the function returns while
box_process_rw expects the statement to be valid until the end of
the transaction. By happy accident, it worked fine until commit
1f7b0d6577f4 ("Require for single statement not autocommit in case of
ddl"), which made it possible to run this function from a transaction
and hence increased the probability of hitting the use-after-free bug.
The fix is trivial: allocate a truncation statement on the region.

Fixes commit 353bcdc5d010 ("Rework space truncation").

Closes #4093
---
https://github.com/tarantool/tarantool/issues/4093
https://github.com/tarantool/tarantool/commits/dv/fix-use-after-free-in-space-truncate

 src/box/box.cc | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 7d890556..7828f575 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1178,21 +1178,25 @@ box_upsert(uint32_t space_id, uint32_t index_id, const char *tuple,
 static void
 space_truncate(struct space *space)
 {
-	char tuple_buf[32];
+	size_t buf_size = 3 * mp_sizeof_array(UINT32_MAX) +
+			  4 * mp_sizeof_uint(UINT64_MAX) + mp_sizeof_str(1);
+	char *buf = (char *)region_alloc_xc(&fiber()->gc, buf_size);
+
+	char *tuple_buf = buf;
 	char *tuple_buf_end = tuple_buf;
 	tuple_buf_end = mp_encode_array(tuple_buf_end, 2);
 	tuple_buf_end = mp_encode_uint(tuple_buf_end, space_id(space));
 	tuple_buf_end = mp_encode_uint(tuple_buf_end, 1);
-	assert(tuple_buf_end < tuple_buf + sizeof(tuple_buf));
+	assert(tuple_buf_end < buf + buf_size);
 
-	char ops_buf[128];
+	char *ops_buf = tuple_buf_end;
 	char *ops_buf_end = ops_buf;
 	ops_buf_end = mp_encode_array(ops_buf_end, 1);
 	ops_buf_end = mp_encode_array(ops_buf_end, 3);
 	ops_buf_end = mp_encode_str(ops_buf_end, "+", 1);
 	ops_buf_end = mp_encode_uint(ops_buf_end, 1);
 	ops_buf_end = mp_encode_uint(ops_buf_end, 1);
-	assert(ops_buf_end < ops_buf + sizeof(ops_buf));
+	assert(ops_buf_end < buf + buf_size);
 
 	if (box_upsert(BOX_TRUNCATE_ID, 0, tuple_buf, tuple_buf_end,
 		       ops_buf, ops_buf_end, 0, NULL) != 0)
-- 
2.11.0

Re: [PATCH] box: fix use-after-free in space_truncate

[Vladimir Davydov]

On Sun, Apr 07, 2019 at 02:24:30PM +0300, Vladimir Davydov wrote:
> space_truncate allocates a statement on the stack which is grossly
> incorrect as the stack may be purged once the function returns while
> box_process_rw expects the statement to be valid until the end of
> the transaction. By happy accident, it worked fine until commit
> 1f7b0d6577f4 ("Require for single statement not autocommit in case of
> ddl"), which made it possible to run this function from a transaction
> and hence increased the probability of hitting the use-after-free bug.
> The fix is trivial: allocate a truncation statement on the region.
> 
> Fixes commit 353bcdc5d010 ("Rework space truncation").
> 
> Closes #4093
> ---
> https://github.com/tarantool/tarantool/issues/4093
> https://github.com/tarantool/tarantool/commits/dv/fix-use-after-free-in-space-truncate
> 
>  src/box/box.cc | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

This one is pretty obvious. Pushed to master and resurrected the
box/transaction test case that triggered the use-after-free bug.

Since the bus was there for two years and we were just lucky not to step
on it, I also cherry-picked this commit to 2.1 and 1.10.