From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f65.google.com (mail-lf1-f65.google.com [209.85.167.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A40BA469719 for ; Tue, 3 Nov 2020 00:18:31 +0300 (MSK) Received: by mail-lf1-f65.google.com with SMTP id k14so4131345lfg.7 for ; Mon, 02 Nov 2020 13:18:31 -0800 (PST) Date: Tue, 3 Nov 2020 00:18:28 +0300 From: Cyrill Gorcunov Message-ID: <20201102211828.GE2339@grain> References: <429b3e4d-490d-ea88-946a-fc0487f9e46a@tarantool.org> <20201102131945.GB517@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org 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? --- From: Cyrill Gorcunov Date: Sun, 1 Nov 2020 16:10:08 +0300 Subject: [PATCH] util: introduce scnprintf helper The misuse of snprintf is pretty common, lets provide scnprintf helper which is just a wrapper over vsnprintf with more sane return. Signed-off-by: Cyrill Gorcunov --- src/lib/core/util.c | 24 ++++++++++++++++++++++++ src/trivia/util.h | 3 +++ 2 files changed, 27 insertions(+) diff --git a/src/lib/core/util.c b/src/lib/core/util.c index dfce317f0..60e552174 100644 --- a/src/lib/core/util.c +++ b/src/lib/core/util.c @@ -84,6 +84,30 @@ strnindex(const char **haystack, const char *needle, uint32_t len, uint32_t hmax return hmax; } +/** + * scnprintf - Put formatted string into a buffer + * + * @param buf The destination buffer + * @param size The size of the buffer + * @param fmt The format string + * @param ... Arguments + * + * @return Number of chars written into \a buf without ending '\0'. + * @return 0 if \a size if 0. + */ +int +scnprintf(char *buf, size_t size, const char *fmt, ...) +{ + va_list args; + int i; + + va_start(args, fmt); + i = vsnprintf(buf, size, fmt, args); + va_end(args); + + return i < (int)size ? i : ((size != 0) ? (int)size-1 : 0); +} + void close_all_xcpt(int fdc, ...) { diff --git a/src/trivia/util.h b/src/trivia/util.h index da5a3705e..cf2da15fc 100644 --- a/src/trivia/util.h +++ b/src/trivia/util.h @@ -98,6 +98,9 @@ strindex(const char **haystack, const char *needle, uint32_t hmax); uint32_t strnindex(const char **haystack, const char *needle, uint32_t len, uint32_t hmax); +int +scnprintf(char *buf, size_t size, const char *fmt, ...); + #define nelem(x) (sizeof((x))/sizeof((x)[0])) #define field_sizeof(compound_type, field) sizeof(((compound_type *)NULL)->field) #ifndef lengthof -- 2.26.2