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.

diff --git a/src/stdio/vsnprintf.c b/src/stdio/vsnprintf.c
index be2c44eb176d70a5d867b9f339dd58afe5953ec7..b3510a63408abe2d4d08e630aa1ddf5b7ba1480a 100644 (file)
--- 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);
 }