Date: Tue, 6 Nov 2012 14:20:53 GMT From: Fabian Keil <fk@fabiankeil.de> To: freebsd-gnats-submit@FreeBSD.org Subject: standards/173421: [patch] strptime() accepts formats that should be rejected Message-ID: <201211061420.qA6EKrYx021552@red.freebsd.org> Resent-Message-ID: <201211061430.qA6EU0ga022785@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 173421
>Category: standards
>Synopsis: [patch] strptime() accepts formats that should be rejected
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-standards
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Tue Nov 06 14:30:00 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator: Fabian Keil
>Release: HEAD
>Organization:
>Environment:
FreeBSD r500.local 10.0-CURRENT FreeBSD 10.0-CURRENT #507 r+e8d2611: Mon Nov 5 13:00:26 CET 2012 fk@r500.local:/usr/obj/usr/src/sys/ZOEY amd64
>Description:
If two digits are followed by a space in buf, %H lets strptime()
parse the two digits as hours which is correct, but then move the
format pointer to the end, thus ignoring the rest of buf and
reporting a successful parse operation without looking at any
additional format specifiers.
This lets the format "%a, %d-%b-%y %H:%M:%S" "match"
"Thursday, 18-Oct-2012 00:11:22", even though the parsing should
fail after %H matched the 12 (because %y only matches the 20).
The resulting time is obviously incorrect.
This affects applications that deal with multiple date variations
by testing several different formats until finding one that is
accepted by strptime(), for example applications that parse dates
in HTTP headers. I noticed the problem while working on Privoxy.
I only confirmed this problem with the %H specifier, but from the
code it looks like other format specifiers are affected as well.
Applications can work around the problem by checking
"%a, %d-%b-%Y %H:%M:%S" before "%a, %d-%b-%y %H:%M:%S",
but this triggers bugs in other strptime() implementations.
The attached patch solves the problem by completely removing the
code that causes the format pointer skipping. This may be incorrect,
but I couldn't think of a scenario where the code is useful, only of
scenarios where it doesn't hurt.
My test case is also handled correctly if the second while()
condition in the removed code is reversed, which might look
reasonable on a first glance, but breaks if %H is followed
by a space and an additional format specifier, in which case
the %H case would move the format pointer to the beginning of
the next format specifier, which would then reject the space
in the buf.
The second patch additionally removes the questionable code for
a couple of other format specifiers, but at least for my use case
the code didn't cause any problems as the skip condition is always
false. Again I couldn't think of a scenario where it's useful,
though.
>How-To-Repeat:
The output of the test case (which I'll submit per mail to keep
the formatting) should be:
Supposedly matching format: %a, %d-%b-%Y %H:%M:%S
Thursday, 18-Oct-2012 00:11:22 GMT -> Thursday, 18-Oct-2012 00:11:22 GMT
ok
Without the patch it's:
Supposedly matching format: %a, %d-%b-%y %H:%M:%S
Thursday, 18-Oct-2012 00:11:22 GMT -> Sunday, 18-Oct-2020 12:00:00 GMT
fail
>Fix:
Apply the attached patch after confirming that the removed code indeed does nothing useful.
Patch attached with submission follows:
>From 6c81c0d84f333075569e84ef9008760bd4689e19 Mon Sep 17 00:00:00 2001
From: Fabian Keil <fk@fabiankeil.de>
Date: Thu, 18 Oct 2012 18:23:00 +0200
Subject: [PATCH 1/2] Fix hour parsing with %H (or break it differently?)
If two digits were followed by a space in buf, %H would parse
the two digits and move the format pointer to the end, thus
reporting a completely successful parse operation without
looking at any additional format specifiers.
This would let the format "%a, %d-%b-%y %H:%M:%S" match
"Thursday, 18-Oct-2012 00:11:22", even though the parsing should
fail after %H matched the 12 (because %y only matches the 20).
---
lib/libc/stdtime/strptime.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/lib/libc/stdtime/strptime.c b/lib/libc/stdtime/strptime.c
index a4ea3fd..3117d3b 100644
--- a/lib/libc/stdtime/strptime.c
+++ b/lib/libc/stdtime/strptime.c
@@ -285,11 +285,6 @@ label:
tm->tm_hour = i;
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 'p':
--
1.7.12.4
>From 7b008a9c6555976d5531b45bc72999c27be7e33e Mon Sep 17 00:00:00 2001
From: Fabian Keil <fk@fabiankeil.de>
Date: Fri, 26 Oct 2012 11:45:02 +0200
Subject: [PATCH 2/2] strptime: Apply the previous fix to the rest of the
format specifiers
Not thoroughly tested.
---
lib/libc/stdtime/strptime.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/lib/libc/stdtime/strptime.c b/lib/libc/stdtime/strptime.c
index 3117d3b..7a03c96 100644
--- a/lib/libc/stdtime/strptime.c
+++ b/lib/libc/stdtime/strptime.c
@@ -248,11 +248,6 @@ label:
tm->tm_sec = i;
}
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 'H':
@@ -354,11 +349,6 @@ label:
if (i > 53)
return 0;
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 'w':
@@ -371,11 +361,6 @@ label:
tm->tm_wday = i;
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 'd':
@@ -403,11 +388,6 @@ label:
tm->tm_mday = i;
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 'B':
@@ -464,11 +444,6 @@ label:
tm->tm_mon = i - 1;
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 's':
@@ -517,11 +492,6 @@ label:
tm->tm_year = i;
- if (*buf != 0 &&
- isspace_l((unsigned char)*buf, locale))
- while (*ptr != 0 &&
- !isspace_l((unsigned char)*ptr, locale))
- ptr++;
break;
case 'Z':
--
1.7.12.4
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211061420.qA6EKrYx021552>