[Tarantool-patches] [PATCH 02/16] Check data_offset overflow in struct tuple
Aleksandr Lyapunov
alyapunov at tarantool.org
Wed Jul 8 18:14:09 MSK 2020
data_offset member of tuple is uint16_t now. At the same time
this field is calculated from field_map_size which is uint32_t.
That could lead to overflows and crashes.
Fixes #5084
---
src/box/errcode.h | 1 +
src/box/memtx_engine.c | 19 ++++++++-----
src/box/tuple.c | 11 ++++++--
src/box/vy_stmt.c | 8 ++++++
test/box/error.result | 1 +
test/box/huge_field_map.result | 49 +++++++++++++++++++++++++++++++++
test/box/huge_field_map.test.lua | 22 +++++++++++++++
test/box/huge_field_map_long.result | 51 +++++++++++++++++++++++++++++++++++
test/box/huge_field_map_long.test.lua | 28 +++++++++++++++++++
test/box/suite.ini | 1 +
10 files changed, 183 insertions(+), 8 deletions(-)
create mode 100644 test/box/huge_field_map.result
create mode 100644 test/box/huge_field_map.test.lua
create mode 100644 test/box/huge_field_map_long.result
create mode 100644 test/box/huge_field_map_long.test.lua
diff --git a/src/box/errcode.h b/src/box/errcode.h
index d1e4d02..938d411 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -266,6 +266,7 @@ struct errcode_record {
/*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with id %u does not exist") \
/*212 */_(ER_SEQUENCE_NOT_STARTED, "Sequence '%s' is not started") \
/*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s doesn't exist") \
+ /*214 */_(ER_TUPLE_METADATA_IS_TOO_BIG, "Can't create tuple: metadata size %u is too big") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index 6ce8cac..b5b6b14 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -1125,6 +1125,18 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end)
if (tuple_field_map_create(format, data, true, &builder) != 0)
goto end;
uint32_t field_map_size = field_map_build_size(&builder);
+ /*
+ * Data offset is calculated from the begin of the struct
+ * tuple base, not from memtx_tuple, because the struct
+ * tuple is not the first field of the memtx_tuple.
+ */
+ uint32_t data_offset = sizeof(struct tuple) + field_map_size;
+ if (data_offset > UINT16_MAX) {
+ /** tuple->data_offset is 16 bits */
+ diag_set(ClientError, ER_TUPLE_METADATA_IS_TOO_BIG,
+ data_offset);
+ goto end;
+ }
size_t tuple_len = end - data;
size_t total = sizeof(struct memtx_tuple) + field_map_size + tuple_len;
@@ -1157,12 +1169,7 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end)
tuple->bsize = tuple_len;
tuple->format_id = tuple_format_id(format);
tuple_format_ref(format);
- /*
- * Data offset is calculated from the begin of the struct
- * tuple base, not from memtx_tuple, because the struct
- * tuple is not the first field of the memtx_tuple.
- */
- tuple->data_offset = sizeof(struct tuple) + field_map_size;
+ tuple->data_offset = data_offset;
char *raw = (char *) tuple + tuple->data_offset;
field_map_build(&builder, raw - field_map_size);
memcpy(raw, data, tuple_len);
diff --git a/src/box/tuple.c b/src/box/tuple.c
index 1f52a8c..e48ee08 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -83,6 +83,13 @@ runtime_tuple_new(struct tuple_format *format, const char *data, const char *end
if (tuple_field_map_create(format, data, true, &builder) != 0)
goto end;
uint32_t field_map_size = field_map_build_size(&builder);
+ uint32_t data_offset = sizeof(struct tuple) + field_map_size;
+ if (data_offset > UINT16_MAX) {
+ /** tuple->data_offset is 16 bits */
+ diag_set(ClientError, ER_TUPLE_METADATA_IS_TOO_BIG,
+ data_offset);
+ goto end;
+ }
size_t data_len = end - data;
size_t total = sizeof(struct tuple) + field_map_size + data_len;
@@ -97,8 +104,8 @@ runtime_tuple_new(struct tuple_format *format, const char *data, const char *end
tuple->bsize = data_len;
tuple->format_id = tuple_format_id(format);
tuple_format_ref(format);
- tuple->data_offset = sizeof(struct tuple) + field_map_size;
- char *raw = (char *) tuple + tuple->data_offset;
+ tuple->data_offset = data_offset;
+ char *raw = (char *) tuple + data_offset;
field_map_build(&builder, raw - field_map_size);
memcpy(raw, data, data_len);
say_debug("%s(%zu) = %p", __func__, data_len, tuple);
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 392f3da..f59c418 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -159,6 +159,14 @@ static struct tuple *
vy_stmt_alloc(struct tuple_format *format, uint32_t data_offset, uint32_t bsize)
{
assert(data_offset >= sizeof(struct vy_stmt) + format->field_map_size);
+
+ if (data_offset > UINT16_MAX) {
+ /** tuple->data_offset is 16 bits */
+ diag_set(ClientError, ER_TUPLE_METADATA_IS_TOO_BIG,
+ data_offset);
+ return NULL;
+ }
+
struct vy_stmt_env *env = format->engine;
uint32_t total_size = data_offset + bsize;
if (unlikely(total_size > env->max_tuple_size)) {
diff --git a/test/box/error.result b/test/box/error.result
index 2196fa5..a166824 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -432,6 +432,7 @@ t;
| 211: box.error.WRONG_QUERY_ID
| 212: box.error.SEQUENCE_NOT_STARTED
| 213: box.error.NO_SUCH_SESSION_SETTING
+ | 214: box.error.TUPLE_METADATA_IS_TOO_BIG
| ...
test_run:cmd("setopt delimiter ''");
diff --git a/test/box/huge_field_map.result b/test/box/huge_field_map.result
new file mode 100644
index 0000000..11b4da3
--- /dev/null
+++ b/test/box/huge_field_map.result
@@ -0,0 +1,49 @@
+-- test-run result file version 2
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+
+s = box.schema.space.create('test', {engine = 'memtx'})
+ | ---
+ | ...
+i1 = s:create_index('pk')
+ | ---
+ | ...
+i2 = s:create_index('mk', {parts={{'[2][*]', 'uint'}}})
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+function test()
+ local t = {1, {}}
+ for i = 1,65536 do
+ table.insert(t[2], i)
+ if (i % 4096 == 0) then
+ s:replace(t)
+ end
+ end
+end;
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | - true
+ | ...
+
+pcall(test) -- must fail but not crash
+ | ---
+ | - false
+ | - 'Can''t create tuple: metadata size 65558 is too big'
+ | ...
+
+test = nil
+ | ---
+ | ...
+s:drop()
+ | ---
+ | ...
diff --git a/test/box/huge_field_map.test.lua b/test/box/huge_field_map.test.lua
new file mode 100644
index 0000000..9042751
--- /dev/null
+++ b/test/box/huge_field_map.test.lua
@@ -0,0 +1,22 @@
+env = require('test_run')
+test_run = env.new()
+
+s = box.schema.space.create('test', {engine = 'memtx'})
+i1 = s:create_index('pk')
+i2 = s:create_index('mk', {parts={{'[2][*]', 'uint'}}})
+test_run:cmd("setopt delimiter ';'")
+function test()
+ local t = {1, {}}
+ for i = 1,65536 do
+ table.insert(t[2], i)
+ if (i % 4096 == 0) then
+ s:replace(t)
+ end
+ end
+end;
+test_run:cmd("setopt delimiter ''");
+
+pcall(test) -- must fail but not crash
+
+test = nil
+s:drop()
\ No newline at end of file
diff --git a/test/box/huge_field_map_long.result b/test/box/huge_field_map_long.result
new file mode 100644
index 0000000..d7971ae
--- /dev/null
+++ b/test/box/huge_field_map_long.result
@@ -0,0 +1,51 @@
+-- test-run result file version 2
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+
+s = box.schema.space.create('test', {engine = 'memtx'})
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+function test()
+ local t = {}
+ local k = {}
+ for i = 1,128 do
+ local parts = {}
+ for j = 0,127 do
+ table.insert(parts, {i * 128 - j, 'uint'})
+ table.insert(t, 1)
+ end
+ if i == 1 then k = table.deepcopy(t) end
+ s:create_index('test'..i, {parts = parts})
+ if i % 16 == 0 then
+ s:replace(t)
+ s:delete(k)
+ end
+ end
+end;
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | - true
+ | ...
+
+pcall(test) -- must fail but not crash
+ | ---
+ | - false
+ | - 'Can''t create tuple: metadata size 65542 is too big'
+ | ...
+
+test = nil
+ | ---
+ | ...
+s:drop()
+ | ---
+ | ...
diff --git a/test/box/huge_field_map_long.test.lua b/test/box/huge_field_map_long.test.lua
new file mode 100644
index 0000000..6415615
--- /dev/null
+++ b/test/box/huge_field_map_long.test.lua
@@ -0,0 +1,28 @@
+env = require('test_run')
+test_run = env.new()
+
+s = box.schema.space.create('test', {engine = 'memtx'})
+test_run:cmd("setopt delimiter ';'")
+function test()
+ local t = {}
+ local k = {}
+ for i = 1,128 do
+ local parts = {}
+ for j = 0,127 do
+ table.insert(parts, {i * 128 - j, 'uint'})
+ table.insert(t, 1)
+ end
+ if i == 1 then k = table.deepcopy(t) end
+ s:create_index('test'..i, {parts = parts})
+ if i % 16 == 0 then
+ s:replace(t)
+ s:delete(k)
+ end
+ end
+end;
+test_run:cmd("setopt delimiter ''");
+
+pcall(test) -- must fail but not crash
+
+test = nil
+s:drop()
\ No newline at end of file
diff --git a/test/box/suite.ini b/test/box/suite.ini
index de8f5a7..801a91e 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -3,6 +3,7 @@ core = tarantool
description = Database tests
script = box.lua
disabled = rtree_errinj.test.lua tuple_bench.test.lua
+long_run = huge_field_map_long.test.lua
config = engine.cfg
release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua gh-4648-func-load-unload.test.lua
lua_libs = lua/fifo.lua lua/utils.lua lua/bitset.lua lua/index_random_test.lua lua/push.lua lua/identifier.lua
--
2.7.4
More information about the Tarantool-patches
mailing list