[PATCH v1 1/1] iproto: fix assertion failure on invalid msgpack

[Kirill Shcherbatov]

https://github.com/tarantool/tarantool/tree/kshch/gh-3900-binary-socket-gibberish-crash
https://github.com/tarantool/tarantool/issues/3900

In some cases, only the initial portion of the transmitted client
request buffer is a valid msgpack. Function row_header_decode
admits such scenario, however, proto_msg_decode used
assert(*pos == reqend) to ensure that the data has been
fully processed. It is wrong.
Based on the error handling policy in this module, let's set diag
message and continue processing the buffer.

Closes #3900
---
 src/box/iproto.cc         |  5 ++++-
 test/box/net.box.result   | 25 +++++++++++++++++++++++++
 test/box/net.box.test.lua | 11 +++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index c8b83b16b..4e08c8e0b 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1159,7 +1159,10 @@ iproto_msg_decode(struct iproto_msg *msg, const char **pos, const char *reqend,
 
 	if (xrow_header_decode(&msg->header, pos, reqend))
 		goto error;
-	assert(*pos == reqend);
+	if (unlikely(*pos != reqend)) {
+		diag_set(ClientError, ER_INVALID_MSGPACK, "packet end");
+		goto error;
+	}
 
 	type = msg->header.type;
 
diff --git a/test/box/net.box.result b/test/box/net.box.result
index b800531b4..9ce6117fc 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')
 ---
 ...
+--
+-- 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') ~= nil
+---
+- true
+...
 -- 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 9e5ecfa0d..8d8257d08 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')
 
+--
+-- 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') ~= nil
+
 -- gh-983 selecting a lot of data crashes the server or hangs the
 -- connection
 
-- 
2.21.0