Re: [tarantool-patches] [PATCH 1/1] iproto: protect from false-correct size in msg header

[Konstantin Osipov <kostja@tarantool.org>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/06/18 22:55]:

Please move the check for buffer overflow to
iproto_enqueue_batch(). We should not pass invalid packet lengths
to iproto_connection_input_buffer().

> Consider this packet:
> 
>     msgpack = require('msgpack')
>     data = msgpack.encode(18400000000000000000)..'aaaaaaa'
> 
> Tarantool interprets 18400000000000000000 as size of a coming
> iproto request, and tries with no any checks to allocate buffer
> of such size. It calculates needed capacity like this:
> 
>     capacity = start_value;
>     while (capacity < size)
>         capacity *= 2;
> 
> Here it is possible that on i-th iteration 'capacity' < 'size',
> but 'capacity * 2' overflows 64 bits and becomes < 'size' again,
> so this loop never ends and occupies 100% CPU.
> 
> Strictly speaking overflow has undefined behavior. On the
> original system it led to nullifying 'capacity'.
> 
> Such size is improbable as a real packet gabarits, but can appear
> as a result of parsing of some invalid packet, first bytes of
> which accidentally appears to be valid MessagePack uint. This is
> how the bug emerged on the real system.
> 
> Lets restrict the maximal packet size to 2GB.
> 
> Closes #3464
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-3464-iproto-100-cpu
> Issue: https://github.com/tarantool/tarantool/issues/3464
> 
>  src/box/iproto.cc         | 15 +++++++++++++--
>  test/box/net.box.result   | 22 ++++++++++++++++++++++
>  test/box/net.box.test.lua | 10 ++++++++++
>  3 files changed, 45 insertions(+), 2 deletions(-)

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov