From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 19 Jun 2018 14:57:40 +0300 From: Vladimir Davydov Subject: Re: [PATCH 1/1] iproto: protect from false-correct size in msg header Message-ID: <20180619115740.ezt2aue5gccoppdv@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org List-ID: Looks OK to me. On Mon, Jun 18, 2018 at 10:53:01PM +0300, Vladislav Shpilevoy wrote: > 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(-)