From: Aleksandr Lyapunov <alyapunov@tarantool.org> To: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v3 02/13] Check data_offset overflow in struct tuple Date: Wed, 15 Jul 2020 16:55:25 +0300 [thread overview] Message-ID: <1594821336-14468-3-git-send-email-alyapunov@tarantool.org> (raw) In-Reply-To: <1594821336-14468-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 ea521aa..3c21375 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -270,6 +270,7 @@ struct errcode_record { /*215 */_(ER_SYNC_MASTER_MISMATCH, "CONFIRM message arrived for an unknown master id %d, expected %d") \ /*216 */_(ER_SYNC_QUORUM_TIMEOUT, "Quorum collection for a synchronous transaction is timed out") \ /*217 */_(ER_SYNC_ROLLBACK, "A rollback for a synchronous transaction is received") \ + /*218 */_(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 8241ec1..cdecdb2 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -436,6 +436,7 @@ t; | 215: box.error.SYNC_MASTER_MISMATCH | 216: box.error.SYNC_QUORUM_TIMEOUT | 217: box.error.SYNC_ROLLBACK + | 218: 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 7f2f5d8..a9ed671 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-15 13:55 UTC|newest] Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-15 13:55 [Tarantool-patches] [PATCH v3 00/13] Transaction engine for memtx engine Aleksandr Lyapunov 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 01/13] Update license file (2020) Aleksandr Lyapunov 2020-07-15 13:55 ` Aleksandr Lyapunov [this message] 2020-07-16 14:27 ` [Tarantool-patches] [PATCH v3 02/13] Check data_offset overflow in struct tuple Nikita Pettik 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 03/13] vinyl: rename tx_manager -> vy_tx_manager Aleksandr Lyapunov 2020-07-15 16:04 ` Nikita Pettik 2020-07-16 8:17 ` Aleksandr Lyapunov 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 04/13] txm: introduce dirty tuples Aleksandr Lyapunov 2020-07-15 16:22 ` Nikita Pettik 2020-07-16 0:05 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 05/13] txm: save txn in txn_stmt Aleksandr Lyapunov 2020-07-15 16:23 ` Nikita Pettik 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 06/13] txm: add TX status Aleksandr Lyapunov 2020-07-15 16:42 ` Nikita Pettik 2020-07-16 0:08 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 07/13] txm: save does_require_old_tuple flag in txn_stmt Aleksandr Lyapunov 2020-07-15 16:49 ` Nikita Pettik 2020-07-16 0:09 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 08/13] txm: introduce tx manager Aleksandr Lyapunov 2020-07-15 16:51 ` Nikita Pettik 2020-07-15 22:01 ` Vladislav Shpilevoy 2020-07-16 0:10 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 09/13] tmx: introduce prepare sequence number Aleksandr Lyapunov 2020-07-15 17:13 ` Nikita Pettik 2020-07-16 0:11 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 10/13] tmx: introduce conflict tracker Aleksandr Lyapunov 2020-07-16 0:16 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 11/13] txm: introduce txm_story Aleksandr Lyapunov 2020-07-16 0:20 ` Vladislav Shpilevoy 2020-07-17 6:16 ` Aleksandr Lyapunov 2020-07-16 22:25 ` Vladislav Shpilevoy 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 12/13] txm: clarify all fetched tuples Aleksandr Lyapunov 2020-07-15 13:55 ` [Tarantool-patches] [PATCH v3 13/13] tmx: use new tx manager in memtx Aleksandr Lyapunov 2020-07-16 22:26 ` Vladislav Shpilevoy 2020-07-17 5:08 ` Aleksandr Lyapunov 2020-07-23 20:58 ` Vladislav Shpilevoy 2020-07-15 15:47 ` [Tarantool-patches] [PATCH v3 00/13] Transaction engine for memtx engine Aleksandr Lyapunov 2020-07-15 16:38 ` Aleksandr Lyapunov 2020-07-15 16:39 ` Aleksandr Lyapunov 2020-07-15 16:40 ` Aleksandr Lyapunov 2020-07-16 0:05 ` Vladislav Shpilevoy
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=1594821336-14468-3-git-send-email-alyapunov@tarantool.org \ --to=alyapunov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v3 02/13] 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