From: Aleksandr Lyapunov <alyapunov@tarantool.org> To: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH 02/15] Check data_offset overflow in struct tuple Date: Fri, 3 Jul 2020 09:33:04 +0300 [thread overview] Message-ID: <1593757997-4145-3-git-send-email-alyapunov@tarantool.org> (raw) In-Reply-To: <1593757997-4145-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-03 6:33 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-03 6:33 [Tarantool-patches] [PATCH 00/15] Transaction engine for memtx engine Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 01/15] Update license file (2020) Aleksandr Lyapunov 2020-07-03 6:33 ` Aleksandr Lyapunov [this message] 2020-07-05 17:03 ` [Tarantool-patches] [PATCH 02/15] Check data_offset overflow in struct tuple Vladislav Shpilevoy 2020-07-06 13:39 ` Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 03/15] tx: introduce dirty tuples Aleksandr Lyapunov 2020-07-05 17:04 ` Vladislav Shpilevoy 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 04/15] vinyl: rename tx_manager -> vy_tx_manager Aleksandr Lyapunov 2020-07-05 17:04 ` Vladislav Shpilevoy 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 05/15] tx: save txn in txn_stmt Aleksandr Lyapunov 2020-07-05 17:04 ` Vladislav Shpilevoy 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 06/15] tx: add TX status Aleksandr Lyapunov 2020-07-05 17:04 ` Vladislav Shpilevoy 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 07/15] tx: save preserve old tuple flag in txn_stmt Aleksandr Lyapunov 2020-07-05 17:05 ` Vladislav Shpilevoy 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 08/15] tx: introduce tx manager Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 09/15] tx: introduce prepare sequence number Aleksandr Lyapunov 2020-07-05 17:05 ` Vladislav Shpilevoy 2020-07-06 13:50 ` Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 10/15] tx: introduce txn_stmt_destroy Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 11/15] tx: introduce conflict tracker Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 12/15] tx: introduce txm_story Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 13/15] tx: indexes Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 14/15] tx: introduce point conflict tracker Aleksandr Lyapunov 2020-07-03 6:33 ` [Tarantool-patches] [PATCH 15/15] tx: use new tx managet in memtx Aleksandr Lyapunov 2020-07-05 17:03 ` [Tarantool-patches] [PATCH 00/15] Transaction engine for memtx engine Vladislav Shpilevoy 2020-07-06 13:29 ` 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=1593757997-4145-3-git-send-email-alyapunov@tarantool.org \ --to=alyapunov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 02/15] 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