mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Fix sscanf
@ 2001-09-24  3:54 Jeff Dike
  2001-09-24  7:13 ` [PATCH] Fix sscanf (the 3rd verse) Paul
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Dike @ 2001-09-24  3:54 UTC (permalink / raw)
  To: torvalds, alan; +Cc: linux-kernel

sscanf double-increments fmt in a couple of places, causing format characters
to be skipped.  Patch follows.

I'm a bit unhappy about the second chunk, but I don't see a cleaner way to
do it offhand.

				Jeff

--- orig/lib/vsprintf.c	Sun Sep 23 19:20:54 2001
+++ 2.4.10/lib/vsprintf.c	Sun Sep 23 21:28:55 2001
@@ -530,7 +530,8 @@
 
 		/* anything that is not a conversion must match exactly */
 		if (*fmt != '%') {
-			if (*fmt++ != *str++)
+		        /* Don't bump fmt because the for header will do it */
+			if (*fmt != *str++)
 				return num;
 			continue;
 		}
@@ -542,6 +543,10 @@
 		if (*fmt == '*') {
 			while (!isspace(*fmt))
 				fmt++;
+			/* Back it up one because the for header will move it
+			 * it forward again
+			 */
+			fmt--;
 			while(!isspace(*str))
 				str++;
 			continue;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix sscanf (the 3rd verse)
  2001-09-24  3:54 [PATCH] Fix sscanf Jeff Dike
@ 2001-09-24  7:13 ` Paul
  2001-09-24 18:05   ` [PATCH] Fix sscanf (more fixes) Paul
  0 siblings, 1 reply; 3+ messages in thread
From: Paul @ 2001-09-24  7:13 UTC (permalink / raw)
  To: linux-kernel

Jeff Dike <jdike@karaya.com>, on Sun Sep 23, 2001 [10:54:10 PM] said:
> sscanf double-increments fmt in a couple of places, causing format characters
> to be skipped.  Patch follows.
> 
> I'm a bit unhappy about the second chunk, but I don't see a cleaner way to
> do it offhand.
> 
> 				Jeff
> 


	Hi.

	I found buffer overruns and other problems when I looked
at this function again in more detail. (%c and %s format parsing
went horribly wrong in initial tests.)  This patch tries to fix
those also. (and avoids the increment/decrement ugly) I have
tested it a little in a userspace program, and it passes uml's
sscanf usage:) Should check it again in the morning... it may not
be perfect, but at least I hope this makes it a little safer.

Paul
set@pobox.com

--- 2.4.9-ac13-user/lib/vsprintf.c.old	Fri Sep 21 19:42:25 2001
+++ 2.4.9-ac13-user/lib/vsprintf.c	Mon Sep 24 02:53:03 2001
@@ -508,6 +508,7 @@
  * @fmt:	format of buffer
  * @args:	arguments
  */
+
 int vsscanf(const char * buf, const char * fmt, va_list args)
 {
 	const char *str = buf;
@@ -515,36 +516,37 @@
 	int num = 0;
 	int qualifier;
 	int base;
-	unsigned int field_width;
+	int field_width = -1;
 	int is_sign = 0;
 
-	for (; *fmt; fmt++) {
+	while(*fmt && *str) {
 		/* skip any white space in format */
-		if (isspace(*fmt)) {
-			continue;
-		}
+		while (isspace(*fmt))
+			++fmt;
 
 		/* anything that is not a conversion must match exactly */
-		if (*fmt != '%') {
+		if (*fmt != '%' && *fmt) {
 			if (*fmt++ != *str++)
 				return num;
 			continue;
 		}
-		++fmt;
+		if (*fmt)
+			++fmt;
+		else
+			return num;
 		
 		/* skip this conversion.
 		 * advance both strings to next white space
 		 */
 		if (*fmt == '*') {
-			while (!isspace(*fmt))
+			while (!isspace(*fmt) && *fmt)
 				fmt++;
-			while(!isspace(*str))
+			while (!isspace(*str) && *str)
 				str++;
 			continue;
 		}
 
 		/* get field width */
-		field_width = 0xffffffffUL;
 		if (isdigit(*fmt))
 			field_width = skip_atoi(&fmt);
 
@@ -557,25 +559,32 @@
 		base = 10;
 		is_sign = 0;
 
-		switch(*fmt) {
+		if (!*fmt || !*str)
+			return num;
+
+		switch(*fmt++) {
 		case 'c':
 		{
 			char *s = (char *) va_arg(args,char*);
+			if (field_width == -1)
+				field_width = 1;
 			do {
 				*s++ = *str++;
-			} while(field_width-- > 0);
+			} while(field_width-- > 0 && *str);
 			num++;
 		}
 		continue;
 		case 's':
 		{
 			char *s = (char *) va_arg(args, char *);
+			if(field_width == -1)
+				field_width = 0x7ffffff;
 			/* first, skip leading white space in buffer */
 			while (isspace(*str))
 				str++;
 
 			/* now copy until next white space */
-			while (!isspace(*str) && field_width--) {
+			while (*str && !isspace(*str) && field_width--) {
 				*s++ = *str++;
 			}
 			*s = '\0';
@@ -617,6 +626,9 @@
 		while (isspace(*str))
 			str++;
 
+		if (!*str)
+			return num;
+
 		switch(qualifier) {
 		case 'h':
 			if (is_sign) {

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix sscanf (more fixes)
  2001-09-24  7:13 ` [PATCH] Fix sscanf (the 3rd verse) Paul
@ 2001-09-24 18:05   ` Paul
  0 siblings, 0 replies; 3+ messages in thread
From: Paul @ 2001-09-24 18:05 UTC (permalink / raw)
  To: linux-kernel; +Cc: alan

	This patch includes my previous modifications and also
corrects behaviour to match sscanf(3) as far as matching white
space and avoiding a false match in some cases.

Paul
set@pobox.com

--- 2.4.9-ac13-user/lib/vsprintf.c.old	Fri Sep 21 19:42:25 2001
+++ 2.4.9-ac13-user/lib/vsprintf.c	Mon Sep 24 13:52:05 2001
@@ -18,6 +18,7 @@
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
+#include <linux/kernel.h>
 
 #include <asm/div64.h>
 
@@ -515,36 +516,44 @@
 	int num = 0;
 	int qualifier;
 	int base;
-	unsigned int field_width;
+	int field_width = -1;
 	int is_sign = 0;
 
-	for (; *fmt; fmt++) {
+	while(*fmt && *str) {
 		/* skip any white space in format */
+		/* white space in format matchs any amount of
+		 * white space, including none, in the input.
+		 */
 		if (isspace(*fmt)) {
-			continue;
+			while (isspace(*fmt))
+				++fmt;
+			while (isspace(*str))
+				++str;
 		}
 
 		/* anything that is not a conversion must match exactly */
-		if (*fmt != '%') {
+		if (*fmt != '%' && *fmt) {
 			if (*fmt++ != *str++)
-				return num;
+				break;
 			continue;
 		}
+
+		if (!*fmt)
+			break;
 		++fmt;
 		
 		/* skip this conversion.
 		 * advance both strings to next white space
 		 */
 		if (*fmt == '*') {
-			while (!isspace(*fmt))
+			while (!isspace(*fmt) && *fmt)
 				fmt++;
-			while(!isspace(*str))
+			while (!isspace(*str) && *str)
 				str++;
 			continue;
 		}
 
 		/* get field width */
-		field_width = 0xffffffffUL;
 		if (isdigit(*fmt))
 			field_width = skip_atoi(&fmt);
 
@@ -557,25 +566,32 @@
 		base = 10;
 		is_sign = 0;
 
-		switch(*fmt) {
+		if (!*fmt || !*str)
+			break;
+
+		switch(*fmt++) {
 		case 'c':
 		{
 			char *s = (char *) va_arg(args,char*);
+			if (field_width == -1)
+				field_width = 1;
 			do {
 				*s++ = *str++;
-			} while(field_width-- > 0);
+			} while(field_width-- > 0 && *str);
 			num++;
 		}
 		continue;
 		case 's':
 		{
 			char *s = (char *) va_arg(args, char *);
+			if(field_width == -1)
+				field_width = INT_MAX;
 			/* first, skip leading white space in buffer */
 			while (isspace(*str))
 				str++;
 
 			/* now copy until next white space */
-			while (!isspace(*str) && field_width--) {
+			while (*str && !isspace(*str) && field_width--) {
 				*s++ = *str++;
 			}
 			*s = '\0';
@@ -617,6 +633,9 @@
 		while (isspace(*str))
 			str++;
 
+		if (!*str || !isdigit(*str))
+			break;
+
 		switch(qualifier) {
 		case 'h':
 			if (is_sign) {

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2001-09-24 18:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-09-24  3:54 [PATCH] Fix sscanf Jeff Dike
2001-09-24  7:13 ` [PATCH] Fix sscanf (the 3rd verse) Paul
2001-09-24 18:05   ` [PATCH] Fix sscanf (more fixes) Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®