diff --git a/src/box/iproto.cc b/src/box/iproto.cc index c8b83b16bd8e866295d9841b39133d5c9117a02a..3b0ba62346f50cf98a0090bae84d87e44dd746e2 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -1157,7 +1157,7 @@ iproto_msg_decode(struct iproto_msg *msg, const char **pos, const char *reqend, { uint8_t type; - if (xrow_header_decode(&msg->header, pos, reqend)) + if (xrow_header_decode(&msg->header, pos, reqend, true)) goto error; assert(*pos == reqend); diff --git a/src/box/vy_run.c b/src/box/vy_run.c index ff39c133daecd0ad8158fc99f24839a5483b9a0d..fae123d8d27187101efdde6d0502533327e477eb 100644 --- a/src/box/vy_run.c +++ b/src/box/vy_run.c @@ -698,7 +698,7 @@ vy_page_xrow(struct vy_page *page, uint32_t stmt_no, const char *data_end = stmt_no + 1 < page->row_count ? page->data + page->row_index[stmt_no + 1] : page->data + page->unpacked_size; - return xrow_header_decode(xrow, &data, data_end); + return xrow_header_decode(xrow, &data, data_end, false); } /* {{{ vy_run_iterator vy_run_iterator support functions */ @@ -832,7 +832,7 @@ vy_page_read(struct vy_page *page, const struct vy_page_info *page_info, struct xrow_header xrow; data_pos = page->data + page_info->row_index_offset; data_end = page->data + page_info->unpacked_size; - if (xrow_header_decode(&xrow, &data_pos, data_end) == -1) + if (xrow_header_decode(&xrow, &data_pos, data_end, true) == -1) goto error; if (xrow.type != VY_RUN_ROW_INDEX) { diag_set(ClientError, ER_INVALID_RUN_FILE, diff --git a/src/box/xlog.c b/src/box/xlog.c index d5de8e6d8f58ef35861e4523639fc3ecd680b24a..692b5f5edbe862e1c8738a94a1cfc88cf45e57e6 100644 --- a/src/box/xlog.c +++ b/src/box/xlog.c @@ -1801,7 +1801,7 @@ xlog_tx_cursor_next_row(struct xlog_tx_cursor *tx_cursor, /* Return row from xlog tx buffer */ int rc = xrow_header_decode(xrow, (const char **)&tx_cursor->rows.rpos, - (const char *)tx_cursor->rows.wpos); + (const char *)tx_cursor->rows.wpos, false); if (rc != 0) { diag_set(XlogError, "can't parse row"); /* Discard remaining row data */ diff --git a/src/box/xrow.c b/src/box/xrow.c index 107009c90a76a6450dc63826e089a3807616fdd4..bddae1d5bf5af049e884d2571b1a90f4c7fc4fd7 100644 --- a/src/box/xrow.c +++ b/src/box/xrow.c @@ -90,7 +90,7 @@ mp_decode_vclock(const char **data, struct vclock *vclock) int xrow_header_decode(struct xrow_header *header, const char **pos, - const char *end) + const char *end, bool end_is_exact) { memset(header, 0, sizeof(struct xrow_header)); const char *tmp = *pos; @@ -170,6 +170,10 @@ xrow_header_decode(struct xrow_header *header, const char **pos, header->body[0].iov_base = (void *) body; header->body[0].iov_len = *pos - body; } + if (end_is_exact && *pos < end) { + diag_set(ClientError, ER_INVALID_MSGPACK, "packet body"); + return -1; + } return 0; } diff --git a/src/box/xrow.h b/src/box/xrow.h index 5317ae22d5c37e46315a6c17f79d82dada158cc6..3a8cba191d7fd18f8dc9f9843bc22909ad64fedd 100644 --- a/src/box/xrow.h +++ b/src/box/xrow.h @@ -128,14 +128,16 @@ xrow_header_encode(const struct xrow_header *header, uint64_t sync, * @param header[out] xrow to fill * @param pos[inout] the start of a packet * @param end the end of a packet - * + * @param end_is_exact if set, raise an error in case the packet + * ends before @end * @retval 0 on success * @retval -1 on error (check diag) - * @post *pos == end on success + * @post *pos <= end on success + * @post *pos == end on success if @end_is_exact is set */ int -xrow_header_decode(struct xrow_header *header, - const char **pos, const char *end); +xrow_header_decode(struct xrow_header *header, const char **pos, + const char *end, bool end_is_exact); /** * DML request. @@ -676,9 +678,9 @@ vclock_follow_xrow(struct vclock* vclock, const struct xrow_header *row) /** @copydoc xrow_header_decode. */ static inline void xrow_header_decode_xc(struct xrow_header *header, const char **pos, - const char *end) + const char *end, bool end_is_exact) { - if (xrow_header_decode(header, pos, end) < 0) + if (xrow_header_decode(header, pos, end, end_is_exact) < 0) diag_raise(); } diff --git a/src/box/xrow_io.cc b/src/box/xrow_io.cc index e9ee6b0c85987d69163f712cf2a65630a3711b71..4b6e68bc1f3e6dd0a1353b92c71d225c7c2162f4 100644 --- a/src/box/xrow_io.cc +++ b/src/box/xrow_io.cc @@ -58,7 +58,8 @@ coio_read_xrow(struct ev_io *coio, struct ibuf *in, struct xrow_header *row) if (to_read > 0) coio_breadn(coio, in, to_read); - xrow_header_decode_xc(row, (const char **) &in->rpos, in->rpos + len); + xrow_header_decode_xc(row, (const char **) &in->rpos, in->rpos + len, + true); } void @@ -89,7 +90,8 @@ coio_read_xrow_timeout_xc(struct ev_io *coio, struct ibuf *in, if (to_read > 0) coio_breadn_timeout(coio, in, to_read, delay); - xrow_header_decode_xc(row, (const char **) &in->rpos, in->rpos + len); + xrow_header_decode_xc(row, (const char **) &in->rpos, in->rpos + len, + true); } diff --git a/test/box/net.box.result b/test/box/net.box.result index b800531b4892cbd51d16278b57c45b606fd25b2d..aecaf9436fc6b3c4490c96c2ac5312535c17726f 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -1381,6 +1381,31 @@ test_run:grep_log("default", "ER_NO_SUCH_PROC") box.schema.user.revoke('guest', 'execute', 'universe') --- ... +-- +-- gh-3900: tarantool can be crashed by sending gibberish to a +-- binary socket +-- +socket = require("socket") +--- +... +sock = socket.tcp_connect(LISTEN.host, LISTEN.service) +--- +... +data = string.fromhex("6783000000000000000000000000000000000000000000800000C8000000000000000000000000000000000000000000FFFF210100373208000000FFFF000055AAEB66486472530D02000000000010A0350001008000001000000000000000000000000000D05700") +--- +... +sock:write(data) +--- +- 104 +... +sock:close() +--- +- true +... +test_run:grep_log('default', 'ER_INVALID_MSGPACK.*') +--- +- 'ER_INVALID_MSGPACK: Invalid MsgPack - packet body' +... -- gh-983 selecting a lot of data crashes the server or hangs the -- connection -- gh-983 test case: iproto connection selecting a lot of data diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua index 9e5ecfa0dd0d55019cbe945f82e52eb7810b90cd..04d6c1903eac7b55655b10016bc50bd3e2f2438a 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -540,6 +540,17 @@ test_run:cmd("setopt delimiter ''"); test_run:grep_log("default", "ER_NO_SUCH_PROC") box.schema.user.revoke('guest', 'execute', 'universe') +-- +-- gh-3900: tarantool can be crashed by sending gibberish to a +-- binary socket +-- +socket = require("socket") +sock = socket.tcp_connect(LISTEN.host, LISTEN.service) +data = string.fromhex("6783000000000000000000000000000000000000000000800000C8000000000000000000000000000000000000000000FFFF210100373208000000FFFF000055AAEB66486472530D02000000000010A0350001008000001000000000000000000000000000D05700") +sock:write(data) +sock:close() +test_run:grep_log('default', 'ER_INVALID_MSGPACK.*') + -- gh-983 selecting a lot of data crashes the server or hangs the -- connection diff --git a/test/unit/xrow.cc b/test/unit/xrow.cc index d7c172955939f84c4f706a6a766dc7f827831cb0..68a3342394affce0a537e2c28391d6bbbe7ceeeb 100644 --- a/test/unit/xrow.cc +++ b/test/unit/xrow.cc @@ -208,7 +208,7 @@ test_xrow_header_encode_decode() char buffer[2048]; char *pos = mp_encode_uint(buffer, 300); is(xrow_header_decode(&header, (const char **) &pos, - buffer + 100), -1, "bad msgpack end"); + buffer + 100, true), -1, "bad msgpack end"); header.type = 100; header.replica_id = 200; @@ -229,7 +229,7 @@ test_xrow_header_encode_decode() begin += fixheader_len; const char *end = (const char *)vec[0].iov_base; end += vec[0].iov_len; - is(xrow_header_decode(&decoded_header, &begin, end), 0, + is(xrow_header_decode(&decoded_header, &begin, end, true), 0, "header decode"); is(header.type, decoded_header.type, "decoded type"); is(header.replica_id, decoded_header.replica_id, "decoded replica_id");