git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: [PATCH ps/stash-in-c] strbuf_vinsertf: provide the correct buffer size to vsnprintf
Date: Sun, 3 Feb 2019 17:51:54 +0100	[thread overview]
Message-ID: <896ae9dd-7ac3-182e-6692-c09bc4864de0@kdbg.org> (raw)

strbuf_vinsertf inserts a formatted string in the middle of an existing
strbuf value. It makes room in the strbuf by moving existing string to
the back, then formats the string to insert directly into the hole.

It uses vsnprintf to format the string. The buffer size provided in the
invocation is the number of characters available in the allocated space
behind the final string. This does not make any sense at all.

Fix it to pass the length of the inserted string plus one for the NUL.
(The functions saves and restores the character that the NUL occupies.)

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 I found this, because in my environment I have to compile with
 SNPRINTF_RETURNS_BOGUS. Our implementation of vsnprintf in
 compat/snprintf.c writes into the end of the buffer unconditionally,
 at a spot that is unrelated to the formatted string, and this leads to
 "BUG: a NUL byte in commit log message not allowed" in some "stash"
 tests.

 strbuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/strbuf.c b/strbuf.c
index bfbbdadbf3..87ecf7f975 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -270,7 +270,7 @@ void strbuf_vinsertf(struct strbuf *sb, size_t pos, const char *fmt, va_list ap)
 	memmove(sb->buf + pos + len, sb->buf + pos, sb->len - pos);
 	/* vsnprintf() will append a NUL, overwriting one of our characters */
 	save = sb->buf[pos + len];
-	len2 = vsnprintf(sb->buf + pos, sb->alloc - sb->len, fmt, ap);
+	len2 = vsnprintf(sb->buf + pos, len + 1, fmt, ap);
 	sb->buf[pos + len] = save;
 	if (len2 != len)
 		BUG("your vsnprintf is broken (returns inconsistent lengths)");
-- 
2.20.1.86.gb0de946387

             reply	other threads:[~2019-02-03 16:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-03 16:51 Johannes Sixt [this message]
2019-02-04  7:25 ` [PATCH ps/stash-in-c] strbuf_vinsertf: provide the correct buffer size to vsnprintf Johannes Sixt
2019-02-04 10:38   ` Johannes Schindelin
2019-02-04 21:13     ` Johannes Sixt
2019-02-05 10:38       ` Johannes Schindelin
2019-02-04 10:54 ` Johannes Schindelin
2019-02-04 21:57 ` Junio C Hamano
2019-02-05 11:06   ` Johannes Schindelin
2019-02-05 18:01     ` Junio C Hamano
2019-02-06 11:41       ` Johannes Schindelin
2019-02-05 20:30     ` Johannes Sixt
2019-02-06 11:56       ` Johannes Schindelin
2019-02-06 18:11         ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=896ae9dd-7ac3-182e-6692-c09bc4864de0@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=ungureanupaulsebastian@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).