From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 25 Jun 2018 16:55:37 +0300 From: Konstantin Osipov Subject: Re: [tarantool-patches] [PATCH 1/1] iproto: protect from false-correct size in msg header Message-ID: <20180625135537.GA25322@chai> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com List-ID: * Vladislav Shpilevoy [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