Fix two rare edge cases:

1. If vasprintf() returns < 0, do not access a NULL pointer in snmprintf(),
and do not free() the pointer returned from vasprintf() because on some
systems other than OpenBSD, it might be a bogus pointer.
2. If vasprintf() returns == 0, return 0 and "" rather than -1 and NULL.

Besides, free(dst) is pointless after failure (not a bug).

One half OK martijn@, the other half OK deraadt@;
committing quickly before people get hurt.

1 files changed, 18 insertions, 10 deletions

diff --git a/usr.bin/ssh/utf8.c b/usr.bin/ssh/utf8.c
index d6089bdecf5..caf789cee65 100644
--- a/usr.bin/ssh/utf8.c
+++ b/usr.bin/ssh/utf8.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: utf8.c,v 1.1 2016/05/25 23:48:45 schwarze Exp $ */
+/* $OpenBSD: utf8.c,v 1.2 2016/05/30 12:05:56 schwarze Exp $ */
 /*
  * Copyright (c) 2016 Ingo Schwarze <schwarze@openbsd.org>
  *
@@ -81,13 +81,15 @@ vasnmprintf(char **str, size_t maxsz, int *wp, const char *fmt, va_list ap)
 	int	 width;	/* Display width of the character wc. */
 	int	 total_width, max_width, print;
 
-	src = dst = NULL;
-	if (vasprintf(&src, fmt, ap) <= 0)
+	src = NULL;
+	if ((ret = vasprintf(&src, fmt, ap)) <= 0)
 		goto fail;
 
 	sz = strlen(src);
-	if ((dst = malloc(sz)) == NULL)
+	if ((dst = malloc(sz)) == NULL) {
+		free(src);
 		goto fail;
+	}
 
 	if (maxsz > INT_MAX)
 		maxsz = INT_MAX;
@@ -191,12 +193,15 @@ vasnmprintf(char **str, size_t maxsz, int *wp, const char *fmt, va_list ap)
 	return ret;
 
 fail:
-	free(src);
-	free(dst);
-	*str = NULL;
 	if (wp != NULL)
 		*wp = 0;
-	return -1;
+	if (ret == 0) {
+		*str = src;
+		return 0;
+	} else {
+		*str = NULL;
+		return -1;
+	}
 }
 
 int
@@ -209,8 +214,11 @@ snmprintf(char *str, size_t sz, int *wp, const char *fmt, ...)
 	va_start(ap, fmt);
 	ret = vasnmprintf(&cp, sz, wp, fmt, ap);
 	va_end(ap);
-	(void)strlcpy(str, cp, sz);
-	free(cp);
+	if (cp != NULL) {
+		(void)strlcpy(str, cp, sz);
+		free(cp);
+	} else
+		*str = '\0';
 	return ret;
 }