247896 - PR_ParseTimeString use PR_Assert and it calls abort on invalid date

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Megath]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040618 MultiZilla/1.6.2.1d
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040618 MultiZilla/1.6.2.1d

PR_ParseTimeString use PR_Assert and it calls abort on invalid date instead of
return PR_FAILURE or smth.

Reproducible: Always
Steps to Reproduce:
1. my chatzilla crashes with mozilla on certain broken irc servers
2.
3.

Actual Results:  
PR_ParseTimeString called abort(); and mozilla/chatzilla exits.

Comment 1

[Megath]

Attachment: patch for mozilla/nsprpub/pr/src/misc/prtime.c

this patch seems to solve problem.

Comment 2

[Megath]

sorry, i forgot about versions.

mozilla-1.7, earlier versions of mozilla did not have this problem.

Comment 3

[Megath]

Attachment: also I guess asserting invalid time zone isnt good idea. patch v2 :)

patch on mozilla/nsprpub/pr/src/misc/prtime.c

Comment 4

[timeless]

Comment on attachment 151346 [details] [diff] [review]
also I guess asserting invalid time zone isnt good idea. patch v2 :)

>                 default:
>-                  PR_ASSERT (0);
>+                  return PR_FAILURE;
this break is now unreachable:
>                   break;
>                 }

>-          PR_ASSERT(tm.tm_month > -1 
>+          if (!(tm.tm_month > -1 
>                                    && tm.tm_mday > 0 
>                                    && tm.tm_hour > -1
>                                    && tm.tm_min > -1
>-                                   && tm.tm_sec > -1);
>+                                   && tm.tm_sec > -1)) return PR_FAILURE;
personally i'd put the return on its own line.

Comment 5

[Darin Fisher]

This sounds serious.  We call PR_ParseTimeString in Necko to parse HTTP date
valued headers.  Couldn't this be used by evil websites to crash Mozilla? :-(

Comment 6

[Asa Dotzler [:asa]]

1.7.5 has shipped. Moving request to 1.7.6.

Comment 7

[Julien Pierre]

It seems to me the problem exists only in debug builds, and therefore isn't
exploitable . But I agree that we should not call PR_Assert on invalid input
data; this should be reserved for supposedly impossible conditions (i.e. bug
detection).

Comment 8

[Daniel Veditz [:dveditz]]

debug only problem doesn't block 1.7.6 --drivers

Comment 9

[Wan-Teh Chang]

Comment on attachment 151346 [details] [diff] [review]
also I guess asserting invalid time zone isnt good idea. patch v2 :)

>--- prtime.old	2004-03-10 04:39:15.000000000 +0600
>+++ prtime.c	2004-06-21 16:03:53.439540832 +0700
>@@ -1506,7 +1506,7 @@
>                 case TT_EET: zone_offset =  2 * 60; break;
>                 case TT_JST: zone_offset =  9 * 60; break;
>                 default:
>-                  PR_ASSERT (0);
>+                  return PR_FAILURE;
>                   break;
>                 }

This assertion should not be removed.  This assertion
detects a bug in the code rather than invalid input
time string.

The other assertion seems to detect invalid input time
string.  If so, it should be replaced by an if statement
with PR_FAILURE return.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 326557 has been marked as a duplicate of this bug. ***

Comment 11

[Alexey Chernyak]

I just encountered this problem on 1.5 Thunderbird non-debug optimised build on Linux.

A mail message was crashing it and preventing me from using mail. I had to remove the message off the server via telnet. So this is pretty much a DoS.

The message contained this header:

Date: Wed, 15 Feb 2006 22: 8: 6  0000

Comment 12

[Alexey Chernyak]

Thunderbird 1.5 (20060207)
Gentoo
linux-2.6.11-mm1
POP3 mail fetch.

(gdb) run /usr/lib/mozilla-thunderbird/thunderbird-bin
Starting program: /usr/lib/mozilla-thunderbird/thunderbird-bin /usr/lib/mozilla-thunderbird/thunderbird-bin
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
[Thread debugging using libthread_db enabled]
[New Thread -1221097248 (LWP 16445)]
[New Thread -1223251024 (LWP 16454)]
[New Thread -1241048144 (LWP 16455)]
[New Thread -1260000336 (LWP 16456)]
[New Thread -1272276048 (LWP 16462)]
[New Thread -1282049104 (LWP 16466)]
Assertion failure: tm.tm_month > -1 && tm.tm_mday > 0 && tm.tm_hour > -1 && tm.tm_min > -1 && tm.tm_sec > -1, at ../../../../mozilla/nsprpub/pr/src/misc/prtime.c:1558

Program received signal SIGABRT, Aborted.
[Switching to Thread -1221097248 (LWP 16445)]
0xffffe410 in __kernel_vsyscall ()

(gdb) bt
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7508d4d in raise () from /lib/tls/libc.so.6
#2  0xb750a363 in abort () from /lib/tls/libc.so.6
#3  0xb7df2e55 in PR_Assert () from /usr/lib/nspr/libnspr4.so
#4  0xb75f24a0 in _IO_list_all () from /lib/tls/libc.so.6
#5  0xb7e15cf0 in ?? () from /usr/lib/nspr/libnspr4.so
#6  0xb7e19600 in ?? () from /usr/lib/nspr/libnspr4.so
#7  0xb7e195cc in ?? () from /usr/lib/nspr/libnspr4.so
#8  0x00000616 in ?? ()
#9  0xb7f72210 in _dl_runtime_resolve () from /lib/ld-linux.so.2
#10 0xb7e068e8 in PR_ParseTimeString () from /usr/lib/nspr/libnspr4.so
#11 0x000007d6 in ?? ()
#12 0x089e6838 in ?? ()
#13 0x08978968 in ?? ()
#14 0x30b1bc58 in ?? ()
#15 0x08b1bc80 in ?? ()
#16 0x088ec2f0 in ?? ()
#17 0xbfd79728 in ?? ()
#18 0x089e5154 in ?? ()
#19 0xffffffff in ?? ()
#20 0x00000016 in ?? ()
#21 0x00000007 in ?? ()
#22 0xffffffff in ?? ()
#23 0xffffff68 in ?? ()
#24 0x00000016 in ?? ()
#25 0x0000000f in ?? ()
#26 0xffffffff in ?? ()
#27 0x00000000 in ?? ()


I'll attach the culprit message. Just openign the file in Thunderbird terminates it.

Comment 13

[Alexey Chernyak]

Attachment: Thunderbird testcase

Comment 14

[Alexey Chernyak]

Attachment: Simplified Thunderbird testcase

Removed everything from the message leaving only the offending Date: header.
Thunderbird still asserts on it.

Comment 15

[Alexey Chernyak]

Doesn't crash Win32 Thunderbird.

Comment 16

[Jure Repinc (JLP)]

I think I have the same problem here:
http://bugs.gentoo.org/show_bug.cgi?id=122164

Comment 17

[timeless]

personally, having rewritten some piece of this in bug 331216 (yes, i've reviewed the patch in this bug, so i really have no business not duping that bug here), i really don't like the patch here.

Comment 18

[Wan-Teh Chang]

Attachment: patch to fix the assertion failure

Please try this patch and see if it fixes the assertion failure
for you.  See bug 331216 comment 13 for a description of this
patch.

Comment 19

[Wan-Teh Chang]

*** Bug 331216 has been marked as a duplicate of this bug. ***

Comment 20

[Wan-Teh Chang]

Attachment: patch to fix the assertion failure (as checked in)

The reviews of this patch were done in bug 331216.

I checked in this patch on the NSPR trunk (NSPR 4.7),
NSPR_4_6_BRANCH (NSPR 4.6.2), NSPRPUB_PRE_4_2_CLIENT_BRANCH
(Mozilla 1.9 alpha), and MOZILLA_1_8_BRANCH (Mozilla 1.8.1).

Comment 21

[Wan-Teh Chang]

Although I marked this bug fixed, I only fixed the assertion
failure in the debug build and made the parsed time valid
(but still incorrect) in the optimized build.

One could go further and make PR_ParseTimeString either
parse an incorrect input string like
"Wed, 15 Feb 2006 22: 8: 6  0000" correctly, or make
PR_ParseTimeString fail on such an incorrect input.

It is not hard to make PR_ParseTimeString allow spaces
in the HH:MM:SS part of a time string, but I am not sure
if we want to allow this kind of incorrect inputs.

It is not hard to make PR_ParseTimeString fail on this
kind of invalid inputs, but it is a lot more work to make
PR_ParseTimeString fail on all invalid inputs because
PR_ParseTimeString seems to have been written to be
permissive.  I suspect that most applications would
like PR_ParseTimeString to return an approximate time
rather than failing.

So, this is why I only went as far as fixing the assertion
failure.  If you think we should do any of these, please
open a new bug.

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 361670 has been marked as a duplicate of this bug. ***

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 364334 has been marked as a duplicate of this bug. ***