Browse Source

fix crash/out-of-bound read in sscanf

commit d6c855caa88ddb1ab6e24e23a14b1e7baf4ba9c7 caused this
"regression", though the behavior was undefined before, overlooking
that f->shend=0 was being used as a sentinel for "EOF" status (actual
EOF or hitting the scanf field width) of the stream helper (shgetc)
functions.

obviously the shgetc macro could be adjusted to check for a null
pointer in addition to the != comparison, but it's the hot path, and
adding extra code/branches to it begins to defeat the purpose.

so instead of setting shend to a null pointer to block further reads,
which no longer works, set it to the current position (rpos). this
makes the shgetc macro work with no change, but it breaks shunget,
which can no longer look at the value of shend to determine whether to
back up. Szabolcs Nagy suggested a solution which I'm using here:
setting shlim to a negative value is inexpensive to test at shunget
time, and automatically re-trips the cnt>=shlim stop condition in
__shgetc no matter what the original limit was.
Rich Felker 5 years ago
parent
commit
8f12c4e110
2 changed files with 3 additions and 2 deletions
  1. 2 1
      src/internal/shgetc.c
  2. 1 1
      src/internal/shgetc.h

+ 2 - 1
src/internal/shgetc.c

@@ -22,7 +22,8 @@ int __shgetc(FILE *f)
 	off_t cnt = shcnt(f);
 	if (f->shlim && cnt >= f->shlim || (c=__uflow(f)) < 0) {
 		f->shcnt = f->buf - f->rpos + cnt;
-		f->shend = 0;
+		f->shend = f->rpos;
+		f->shlim = -1;
 		return EOF;
 	}
 	cnt++;

+ 1 - 1
src/internal/shgetc.h

@@ -26,7 +26,7 @@ hidden int __shgetc(FILE *);
 #define shcnt(f) ((f)->shcnt + ((f)->rpos - (f)->buf))
 #define shlim(f, lim) __shlim((f), (lim))
 #define shgetc(f) (((f)->rpos != (f)->shend) ? *(f)->rpos++ : __shgetc(f))
-#define shunget(f) ((f)->shend ? (void)(f)->rpos-- : (void)0)
+#define shunget(f) ((f)->shlim>=0 ? (void)(f)->rpos-- : (void)0)
 
 #define sh_fromstring(f, s) \
 	((f)->buf = (f)->rpos = (void *)(s), (f)->rend = (void*)-1)