about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRich Felker <dalias@aerifal.cx>2016-10-21 20:57:15 -0400
committerRich Felker <dalias@aerifal.cx>2016-10-21 22:15:31 -0400
commited8696742504b6182954254e78ec123c8bcd8d3b (patch)
treec48f0867c50e5c1ef2b0104d3d63d675fa29c22c
parent7597fc25a2743d49500926a286da71f8e033936c (diff)
downloadmusl-ed8696742504b6182954254e78ec123c8bcd8d3b.tar.gz
musl-ed8696742504b6182954254e78ec123c8bcd8d3b.tar.xz
musl-ed8696742504b6182954254e78ec123c8bcd8d3b.zip
redesign snprintf without undefined behavior
the old snprintf design setup the FILE buffer pointers to point
directly into the destination buffer; if n was actually larger than
the buffer size, the pointer arithmetic to compute the buffer end
pointer was undefined. this affected sprintf, which is implemented in
terms of snprintf, as well as some unusual but valid direct uses of
snprintf.

instead, setup the FILE as unbuffered and have its write function
memcpy to the destination. the printf core sets up its own temporary
buffer for unbuffered streams.
-rw-r--r--src/stdio/vsnprintf.c63
1 files changed, 38 insertions, 25 deletions
diff --git a/src/stdio/vsnprintf.c b/src/stdio/vsnprintf.c
index be2c44eb..b3510a63 100644
--- a/src/stdio/vsnprintf.c
+++ b/src/stdio/vsnprintf.c
@@ -4,39 +4,52 @@
 #include <errno.h>
 #include <stdint.h>
 
+struct cookie {
+	char *s;
+	size_t n;
+};
+
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
 static size_t sn_write(FILE *f, const unsigned char *s, size_t l)
 {
-	size_t k = f->wend - f->wpos;
-	if (k > l) k = l;
-	memcpy(f->wpos, s, k);
-	f->wpos += k;
-	/* pretend to succeed, but discard extra data */
+	struct cookie *c = f->cookie;
+	size_t k = MIN(c->n, f->wpos - f->wbase);
+	if (k) {
+		memcpy(c->s, f->wbase, k);
+		c->s += k;
+		c->n -= k;
+	}
+	k = MIN(c->n, l);
+	if (k) {
+		memcpy(c->s, s, k);
+		c->s += k;
+		c->n -= k;
+	}
+	*c->s = 0;
+	f->wpos = f->wbase = f->buf;
+	/* pretend to succeed, even if we discarded extra data */
 	return l;
 }
 
 int vsnprintf(char *restrict s, size_t n, const char *restrict fmt, va_list ap)
 {
-	int r;
-	char b;
-	FILE f = { .lbf = EOF, .write = sn_write, .lock = -1 };
+	unsigned char buf[1];
+	char dummy[1];
+	struct cookie c = { .s = n ? s : dummy, .n = n ? n-1 : 0 };
+	FILE f = {
+		.lbf = EOF,
+		.write = sn_write,
+		.lock = -1,
+		.buf = buf,
+		.cookie = &c,
+	};
 
-	if (n-1 > INT_MAX-1) {
-		if (n) {
-			errno = EOVERFLOW;
-			return -1;
-		}
-		s = &b;
-		n = 1;
+	if (n > INT_MAX) {
+		errno = EOVERFLOW;
+		return -1;
 	}
 
-	/* Ensure pointers don't wrap if "infinite" n is passed in */
-	if (n > (char *)0+SIZE_MAX-s-1) n = (char *)0+SIZE_MAX-s-1;
-	f.buf_size = n;
-	f.buf = f.wpos = (void *)s;
-	f.wbase = f.wend = (void *)(s+n);
-	r = vfprintf(&f, fmt, ap);
-
-	/* Null-terminate, overwriting last char if dest buffer is full */
-	if (n) f.wpos[-(f.wpos == f.wend)] = 0;
-	return r;
+	*c.s = 0;
+	return vfprintf(&f, fmt, ap);
 }