[9fans] snprintf buffer overrun

[9fans] snprintf buffer overrun

[Porlock]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

Plan 9's implementation of the standard C functions snprintf and
vsnprintf have a buffer overrun bug.

If the buffer length equals the output length (without the terminating
null), then one too many characters is written to the buffer.

For example,
              snprintf(buf, 4, "ABCD");

will write 5 characters to buf.

Attached is a short program to illustrate this, which gives the
following output :-

% 8c printftest.c && 8l printftest.8
% ./8.out
  A   B   C   D   \0  *   *   *   *   *   *   *   *   *   *   *



[-- Attachment #2: printftest.c --]
[-- Type: text/x-csrc, Size: 341 bytes --]

#include <u.h>
#include <libc.h>
#include <stdio.h>

void main()
{
    char buf[16];
    int i;
    memset(buf, '*', sizeof(buf));
    snprintf(buf, 4, "ABCD");
    for (i = 0; i < sizeof(buf); ++i) {
        if (buf[i])
            print("  %c ", buf[i]);
        else
            print("  \\0");
    }
    print("\n");
}

Re: [9fans] snprintf buffer overrun

[cinap_lenrek]

theres a bug is in sclose() where it doesnt check if wp is beyond
the buffer. also wp was not updated after realloc().

--- a/sys/src/libstdio/sclose.c	Sat Nov 19 16:47:21 2016 +0100
+++ b/sys/src/libstdio/sclose.c	Sun Nov 27 21:07:48 2016 +0100
@@ -5,27 +5,35 @@
 char *sclose(FILE *f){
 	switch(f->state){
 	default:	/* ERR CLOSED */
+	Error:
 		if(f->buf && f->flags&BALLOC)
 			free(f->buf);
-		f->flags=0;
-		return NULL;
+		f->buf=0;
+		break;
 	case OPEN:
 		f->buf=malloc(1);
 		f->buf[0]='\0';
 		break;
 	case RD:
 	case END:
-		f->flags=0;
 		break;
 	case RDWR:
 	case WR:
+		f->rp=f->buf+f->bufl;
 		if(f->wp==f->rp){
-			if(f->flags&BALLOC)
-				f->buf=realloc(f->buf, f->bufl+1);
-			if(f->buf==NULL) return NULL;
+			if(f->flags&BALLOC){
+				char *t = realloc(f->buf, f->bufl+1);
+				if(t==NULL)
+					goto Error;
+				f->buf=t;
+				f->wp=t+f->bufl;
+			} else {
+				if(f->wp > f->buf)
+					*(f->wp-1) = '\0';
+				goto Error;
+			}
 		}
 		*f->wp='\0';
-		f->flags=0;
 		break;
 	}
 	f->state=CLOSED;

--
cinap

Re: [9fans] snprintf buffer overrun

[cinap_lenrek]

commited fix in 9front, thanks for reporting!

--
cinap