From: Aleksandr Lyapunov <alyapunov@tarantool.org>
To: tarantool-patches@dev.tarantool.org
Cc: v.shpilevoy@tarantool.org
Subject: [Tarantool-patches] [PATCH 02/16] Check data_offset overflow in struct tuple
Date: Wed, 8 Jul 2020 18:14:09 +0300 [thread overview]
Message-ID: <1594221263-6228-3-git-send-email-alyapunov@tarantool.org> (raw)
In-Reply-To: <1594221263-6228-1-git-send-email-alyapunov@tarantool.org>
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
next prev parent reply other threads:[~2020-07-08 15:14 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-08 15:14 [Tarantool-patches] [PATCH v2 00/16] Transaction engine for memtx engine Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 01/16] Update license file (2020) Aleksandr Lyapunov
2020-07-08 15:14 ` Aleksandr Lyapunov [this message]
2020-07-12 17:15 ` [Tarantool-patches] [PATCH 02/16] Check data_offset overflow in struct tuple Vladislav Shpilevoy
2020-07-14 17:09 ` Aleksandr Lyapunov
2020-07-14 22:48 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 03/16] tx: introduce dirty tuples Aleksandr Lyapunov
2020-07-12 17:15 ` Vladislav Shpilevoy
2020-07-12 22:24 ` Nikita Pettik
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 04/16] vinyl: rename tx_manager -> vy_tx_manager Aleksandr Lyapunov
2020-07-12 17:14 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 05/16] tx: save txn in txn_stmt Aleksandr Lyapunov
2020-07-12 17:15 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 06/16] tx: add TX status Aleksandr Lyapunov
2020-07-12 17:15 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 07/16] tx: save preserve old tuple flag in txn_stmt Aleksandr Lyapunov
2020-07-12 17:14 ` Vladislav Shpilevoy
2020-07-14 23:46 ` Vladislav Shpilevoy
2020-07-15 7:53 ` Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 08/16] tx: introduce tx manager Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 09/16] tx: introduce prepare sequence number Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 10/16] tx: introduce txn_stmt_destroy Aleksandr Lyapunov
2020-07-12 17:15 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 11/16] tx: introduce conflict tracker Aleksandr Lyapunov
2020-07-12 17:15 ` Vladislav Shpilevoy
2020-07-14 23:51 ` Vladislav Shpilevoy
2020-07-15 7:57 ` Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 12/16] introduce tuple smart pointers Aleksandr Lyapunov
2020-07-12 17:16 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 13/16] tx: introduce txm_story Aleksandr Lyapunov
2020-07-12 17:14 ` Vladislav Shpilevoy
2020-07-14 23:46 ` Vladislav Shpilevoy
2020-07-15 8:11 ` Aleksandr Lyapunov
2020-07-15 22:02 ` Vladislav Shpilevoy
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 14/16] tx: indexes Aleksandr Lyapunov
2020-07-14 23:50 ` Vladislav Shpilevoy
2020-07-15 10:02 ` Aleksandr Lyapunov
2020-07-15 22:08 ` Vladislav Shpilevoy
2020-07-15 10:19 ` Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 15/16] tx: introduce point conflict tracker Aleksandr Lyapunov
2020-07-08 15:14 ` [Tarantool-patches] [PATCH 16/16] tx: use new tx manager in memtx Aleksandr Lyapunov
2020-07-14 23:45 ` Vladislav Shpilevoy
2020-07-15 10:32 ` Aleksandr Lyapunov
2020-07-15 22:09 ` Vladislav Shpilevoy
2020-07-12 17:19 ` [Tarantool-patches] [PATCH v2 00/16] Transaction engine for memtx engine Vladislav Shpilevoy
2020-07-14 23:47 ` Vladislav Shpilevoy
2020-07-15 12:25 ` Aleksandr Lyapunov
2020-07-15 22:10 ` Vladislav Shpilevoy
2020-07-16 4:48 ` Aleksandr Lyapunov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1594221263-6228-3-git-send-email-alyapunov@tarantool.org \
--to=alyapunov@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--cc=v.shpilevoy@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH 02/16] Check data_offset overflow in struct tuple' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox