From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 8CD4B469719 for ; Tue, 3 Nov 2020 00:43:04 +0300 (MSK) References: <429b3e4d-490d-ea88-946a-fc0487f9e46a@tarantool.org> <20201102131945.GB517@tarantool.org> <20201102211828.GE2339@grain> From: Vladislav Shpilevoy Message-ID: Date: Mon, 2 Nov 2020 22:43:02 +0100 MIME-Version: 1.0 In-Reply-To: <20201102211828.GE2339@grain> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] core: fix static_alloc buffer overflow List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org On 02.11.2020 22:18, Cyrill Gorcunov wrote: > On Mon, Nov 02, 2020 at 10:09:29PM +0100, Vladislav Shpilevoy wrote: >>> Thanks for the investigation! My bad, I used MIN as a function with >>> sematics of all agruments calculated before call. You're right - in case >>> of define it can cause a double call. >>> >>> The SNPRINT although leaves some questions to me: in case 'written' is >>> more or equal to 'size', it forces '_buf' to be set to NULL. But in the >>> sio_socketname_to_buffer() there's no check for NULL between the calls. >>> A call to snprintf() delivers a segfault, at least for Mac. >> >> Woops, SNPRINT is used a lot in the code. If it is true, we need to fix SNPRINT. > > Guys, I didn't follow the details of the series but thought of some > helper like below. Will it help? In some places yes. But SNPRINT is used not only with snprintf. It also is used with vsnprintf, mp_snprint, vy_run_snprint_filename, tuple_snprint, say_format_plain_tail, json_escape, strftime, and probably more. So it would be better to fix SNPRINT. To cover all its usage cases.