Bug 13901

    Whenever a numeric data is expected by strptime() in it's `buf' argument,
    it will allocate too many digits to any field which is followed by a
    digit or string of digits in `buf'.

Fix: 

170c170,171
< 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
---
> 			len = 0;
> 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf) && len < 3; buf++) {
172a174
> 				len++;
187,188c189,190
< 
< 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
---
> 			len = 0;
> 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf) && len < 2; buf++) {
190a193
> 				len++;
211,212c214,215
< 
< 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
---
> 			len = 0;
> 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf) && len < 2; buf++) {
214a218
> 				len++;
278,279c282,283
< 
< 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
---
> 			len = 0;
> 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf) && len < 2; buf++) {
281a286
> 				len++;
319,320c324,325
< 
< 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
---
> 			len = 0;
> 			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf) && len < 2; buf++) {
322a328
> 				len++;
341a348
> 			len = 0;
344a352,356
> 				if ((c == 'y' && len >= 1) || (c == 'Y' && len >= 3)) {
> 					buf++;
> 					break;
> 				}
> 				len++;
How-To-Repeat:     #include <time.h>
    struct tm t;
    strptime("19990823", "%Y%m%d", &t); /* fails */
    strptime("199911", "%Y11", &t);  /* fails */
    strptime("0711", "%m%d", &t); /* fails */

The file to patch is /usr/src/lib/libc/stdtime/strptime.c. Sorry I
left that out originally.

scott

This How-To-Repeat is more visually effective. :-)

Ciao,
Sheldon.


#include <stdio.h>
#include <time.h>

struct tm t1, t2, t3;

int
main(void)
{
	strptime("19990823", "%Y%m%d", &t1); /* fails */
	strptime("0711", "%m%d", &t2); /* fails */
	strptime("199911", "%Y11", &t3);  /* fails */

	printf("19990823 -> %%Y%%m%%d:\n"
	      "tm_year   == %d\n"
	      "tm_mon    == %d\n"
	      "tm_mday   == %d\n\n", t1.tm_year, t1.tm_mon, t1.tm_mday);
	printf("    0711 -> %%m%%d:\n"
	       "tm_mon   == %d\n"
	       "tm_mday  == %d\n\n", t2.tm_mon, t2.tm_mday);
	printf("  199911 -> %%Y11:\n"
	       "tm_year  == %d\n", t3.tm_year);

	return 0;
}

Comment 4 Sheldon Hearn 1999-12-03 15:09:48 UTC

Responsible Changed
From-To: freebsd-bugs->sheldonh

I'll get this one sorted out next week if someone doesn't beat me 
to it.  The originator is making me swallow my words in  
freebsd-security Message-ID: <67349.944133898@axl.noc.iafrica.com> 
:-) 

On Wed, 08 Dec 1999 07:33:15 EST, scott wrote:

> I do think that it is better to bound %l and %e at 2 digits than
> to not bound them at all, as long strings of numbers could have
> odd effects otherwise, but I have not offerred a way to make it
> universally understand the fields that have variable numbers of
> digits.  That would be quite tricky, I believe...

Indeed it would, in which case, an XXX-style comment is in order. :-)

Here's what I think is a more comprehensive fix for the general problem
of strptime(3) format specifiers gobbling extra digits.

Note that this diff is against CURRENT, not STABLE, since that's where
it'll be committed first.

Ciao,
Sheldon.

Index: strptime.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdtime/strptime.c,v
retrieving revision 1.15
diff -u -d -r1.15 strptime.c
--- strptime.c	1999/12/08 11:11:40	1.15
+++ strptime.c	1999/12/08 13:34:51
@@ -129,9 +129,12 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			/* XXX This will break for 3-digit centuries. */
+			len = 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (i < 19)
 				return 0;
@@ -206,9 +209,11 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = 3;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (i < 1 || i > 366)
 				return 0;
@@ -224,9 +229,11 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 
 			if (c == 'M') {
@@ -248,12 +255,22 @@
 		case 'I':
 		case 'k':
 		case 'l':
+			/*
+			 * Of these, %l is the only specifier explicitly
+			 * documented as not being zero-padded.  However,
+			 * there is no harm in allowing zero-padding.
+			 *
+			 * XXX The %l specifier may gobble one too many
+			 * digits if used incorrectly.
+			 */
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (c == 'H' || c == 'k') {
 				if (i > 23)
@@ -326,9 +343,11 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (i > 53)
 				return 0;
@@ -342,10 +361,7 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
-				i *= 10;
-				i += *buf - '0';
-			}
+			i = *buf - '0';
 			if (i > 6)
 				return 0;
 
@@ -358,12 +374,18 @@
 
 		case 'd':
 		case 'e':
+			/*
+			 * XXX The %e specifier may gobble one too many
+			 * digits if used incorrectly.
+			 */
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (i > 31)
 				return 0;
@@ -414,9 +436,11 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (i < 1 || i > 12)
 				return 0;
@@ -436,9 +460,11 @@
 			if (!isdigit((unsigned char)*buf))
 				return 0;
 
-			for (i = 0; *buf != 0 && isdigit((unsigned char)*buf); buf++) {
+			len = (c == 'Y') ? 4 : 2;
+			for (i = 0; len && *buf != 0 && isdigit((unsigned char)*buf); buf++) {
 				i *= 10;
 				i += *buf - '0';
+				len--;
 			}
 			if (c == 'Y')
 				i -= 1900;

Here's the manual page update to go with the source code changes.

Ciao,
Sheldon.

Index: strptime.3
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdtime/strptime.3,v
retrieving revision 1.6
diff -u -d -r1.6 strptime.3
--- strptime.3	1999/08/28 00:01:44	1.6
+++ strptime.3	1999/12/08 15:29:13
@@ -95,6 +95,45 @@
 .Pp
 .Sh BUGS
 The
+.Fa %C
+format specifier only accepts centuries within the range 19 to 99.
+.Pp
+Both the
+.Fa %e
+and
+.Fa %l
+format specifiers may incorrectly scan one too many digits
+if the intended values comprise only a single digit
+and that digit is followed immediately by another digit.
+Both specifiers accept zero-padded values,
+even though they are both defined as taking unpadded values.
+.Pp
+The
+.Fa %p
+format specifier has no effect unless it is parsed
+.Em after
+hour-related specifiers.
+Specifying
+.Fa %l
+without
+.Fa %p
+will produce undefined results.
+Note that 12AM
+.Pq ante meridiem
+is taken as midnight
+and 12PM
+.Pq post meridiem
+is taken as noon.
+.Pp
+The
+.Fa %U
+and
+.Fa %W
+format specifiers accept any value within the range 00 to 53
+without validating against other values supplied (like month
+or day of the year, for example).
+.Pp
+The
 .Fa %Z
 format specifier only accepts time zone abbreviations of the local time zone,
 or the value "GMT".

Comment 7 Sheldon Hearn 1999-12-08 15:52:48 UTC

State Changed
From-To: open->suspended

Committed to CURRENT in: 

rev 1.16	src/lib/libc/stdtime/strptime.c 
rev 1.7		src/lib/libc/stdtime/strptime.3 

I'm leaving this in the correct state for a PR awaiting MFC, although 
I will plead with jkh for an early MFC before 3.4-RELEASE. 

Comment 8 Sheldon Hearn 1999-12-09 09:05:47 UTC

State Changed
From-To: suspended->closed

Merged to RELENG_3 as rev 1.4.2.4 of strptime.c. 
Thanks for your patience, Scott! :-)