156725 - citibank.com - let you login, but after that any link report timeout

Status: VERIFIED FIXED

(Tech Evangelism Graveyard :: English US, defect, P1)

Description

[Vadim Kutsyy]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1a+) Gecko/20020708
BuildID:    2002070813

Citibank let you login, but after that any link report timeout.
Citibank works fine with 1.0 and 1.1alpha build

Reproducible: Always
Steps to Reproduce:
1. login to citibankonle
2. click on "Account Information"

Actual Results:  
------------------------------
We're sorry, but your access to this part of the site has timed out.

The site has been built to time out after a period of inactivity, to protect
your account information. 
------------------------------

Expected Results:  Account infomation

This worked fine with 1.0 and 1.1.alpha build.  It also worled fine with night
build at about 6/15/02 (sorry bon't have build #).  It still works fine with IE.
 This error appears in both my work and my home computers (both win2k, same
build ID).  I can add source of the pages, if needed.

Comment 1

[Pascal Chevrel:pascalc]

I thinj that this is a duplicate of bug 85557

Comment 2

[Vadim Kutsyy]

I don't think so.  bug 85557 says that javascript doesn't work at all.  Here is 
seems that there is a problem with cookies, or access to cookies.  My 
understanding is, that time information is stored in cookies.

Comment 3

[Vadim Kutsyy]

Well, it has been resolved.

Citibank changes there webintephaze yestarday, and new one works.  There is
still be a problem with not displaying Account Page, but this is related to bug
85557. 

Comment 4

[Vadim Kutsyy]

Citibank went back to older design, and the problem exist again.

Comment 5

[Jon Baumgartner]

This is happening to me on OS X, so it's not just a Windows-specific bug.

Comment 6

[Mel Reyes]

This was working again shortly after this bug report and then around the
27th/28th it started to timeout again, seems like some kind of regression in
Mozilla worked fine during 1.1 prep and freeze.  I'm using Windows XP and the
latest build of 083008.

Comment 7

[Mel Reyes]

Is anyone going to work on this, this is definitely a regression, to say the
least very annoying.  All my other ssl/javascript site work just fine, it's on
CitiBank's that derives a timeout immediately after loggin into the Direct
Access section.

Will anyone be working on this before 1.2 gets released, locked down for release?

Comment 8

[Bob Clary [:bc] (inactive)]

This is not evangelism. -> browser

Comment 9

[Mel Reyes]

Any luck with this, seems to be a similar dialog toward the end of this bug -->
85557.  This really cripples my Moz usage since I do all my banking online, thanks.

Comment 10

[Mel Reyes]

Not really sure how to get this higher in priority, but this definitely seems to
be a 1.2alpha regression.  Thanks.

Comment 11

[Matthias Versen [:Matti]]

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

Comment 12

[Matthias Versen [:Matti]]

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

Comment 13

[Matthias Versen [:Matti]]

-> default owner + a "few" keywords (note : i have no Account there)

Comment 14

[Susie Wyshak]

Please treat this as a STOP SHIP bug for new US browser releases. Thanks.

Steve please work with Marina if you need to test logins. 

Comment 15

[Jaime Rodriguez, Jr.]

Marking as topembed+/nsbeta1+ as this is a significant and visible regression of
a top banking site, from a previous release.

Comment 16

[Stephen P. Morse]

Waiting for personal response from either marina or from jj with a login.

Comment 17

[Mel Reyes]

Thanks for the great response on this, just some more info in case it helps.

Every since the Moz 1.2 daily builds got opened up this error has come up, it
was there for a very short time with the 1.1 builds and then it went away.  I
think it was around August 24th-27th daily builds that this started happening,
in case that helps.

Now I have to resort to using either IE or NS 7 to login.  Thanks again.

Comment 18

[dan]

Version 1.0.1 build 2002082606 works fine.

Comment 19

[J.J. Enser]

I've seen this using 7.0 RTM for a while, but not anymore. They must have fixed
it on their end in the past few days.

Something slightly related that I still see once a secure session has been
opened on citibank.com is that I get immediately kicked out (or timed out) if I
use the "back" button of the browser. In the past, navigation buttons never
worked, but they were simply refreshing the current page, without closing the
session.
(they have their own navigation bar on each page, which works)

for info, I'm running 7.0 RTM on MacOS X. Let me know if you'd like me to do
some specific testing on that site. You can also come by and see me for "live
testing" if you want.

Comment 20

[Stephen P. Morse]

At first I thought that this was a consequence of bringing our path-testing for
cookies in line with the RFC2109 spec.  See bug 155083 comment 18.  We knew we
were going to break some cites and we needed to find out which ones.

Indeed citibank is setting some cookies that it shouldn't be.  For example the
citibank host at

   https://web.da-us.citibank.com
          /cgi-bin/citifi/scripts/login/user_agreement.jsp

is attempting to set the following cookies:

   TPI=...; path=/ghs; domain=.da-us.citibank.com

   oatmeal=...; expires=Sat, 20-Sep-03 21:24:00 GMT;
   path=/signin; domain=.da-us.citibank.com"

   UAA=...; expires=Sat, 20-Sep-03 21:24:00 GMT;
   path=/signin; domain=.da-us.citibank.com"

   quicklink=...; expires=Sat, 20-Sep-03 21:24:00 GMT; path=/signin"

Note that each of the above cookies are for a path that the setting site does
not have access to.

However, I backed out the patch for 155083 but no luck.  The cookies were now
getting set but we still had the symptoms described in this report.

Still investigating.

Comment 21

[Stephen P. Morse]

This one is proving extremely difficult to zero in on, due to the extremely
large number of cookies that this site is setting.  It would help if I had some
cooperation from the citibank technical people to find out which cookie they are
not liking.  I called them but they were less than cooperative (we don't have a
big enough market share).

I have found a few problem so far, but even with those corrected the site is
still not working.  What I have found is:

1. Site is setting cookie paths in violation of the cookie spec (see comment20).
2. We are confusing session cookies with expired cookies.  The fix for this is:

   in COOKIE_SetCookieStringFromHttp add the following:

    expires = cookie_ParseDate(date);
+   if (expires == 0) {
+     expires = 1; // avoid confusion with session cookies
+   }

Comment 22

[Susie Wyshak]

Does this Ebay/Passport issue look like a dupe? bug 170396
If so please mark it. Thanks.

Comment 23

[Susie Wyshak]

Steve we will try to get someone to work with you.

Comment 24

[Stephen P. Morse]

After some very tedious debugging, I finally found another problem with the
site.  Namely the site was setting a cookie as follows:

   cookie name = TPI
   path = /ghs

and then requests were made from the following sites:

   t9863624.da-us.citibank.com/ghs/PHP/?D=M&C=DC3&_dt=1032869276851
   t9863624.da-us.citibank.com/ghs?D=M&C=P

In the first case the path matches and the cookie should be sent.  It was.  In
the second case the path does not match (ghs is the file name and not the patyh)
and the cookie was not being sent.  But apparently the site was expecting that
the cookie be sent.  If I put in a band-aid and sent the cookie, everything
worked perfectly.

OK, now I have three band-aids to fix the three problems encountered.  And with
those band-aids everything works.  So now I have to determine which ones are the
crucial ones, remove the unnecessary band-aids, and get good patches for the
necessary ones.  This is the easy part -- the analysis was the hard part and
that is now completed.

Comment 25

[Stephen P. Morse]

Unfortunately citibank is getting upset with all the testing that I'm doing, and
they have deactivated the account I was using.  The person whose account it was
is unhappy about the inconvenience.

So I am no longer able to test this out.  I am attaching what I believe may
allow us to log onto the citibank site, but it is untested.  It has fixes for
all three of the problems I've identified.  I'm not sure that the timer problem
is really a problem, but without testing I'm not able to find out.

If someone can do some evangelism work here and get us a test account, that
would be terrific.

Comment 26

[Stephen P. Morse]

Attachment: Untested patch that might work

Comment 27

[Boris Zbarsky [:bzbarsky]]

I feel that commenting out the path check (the middle chunk) just means that we
open up sites and users to security/privacy issues forever.  If there's no
problem, the sites in question will never change their code -- there will be
absolutely no incentive to.  So "until these sites are evangelized" just means
"till either we or they are gone".... unfortunately, them being gone will not
help as other sites will use the broken syntax if all browsers support it....

Comment 28

[Stephen P. Morse]

As I said, patch is untested.  Eyeballing it, I see an error in the logic.  The
following if statement

   if (pathLen>cookiePathLen &&
       (path[cookiePathLen] != '/' || path[cookiePathLen] != '?')) {

should instead read

   if (pathLen>cookiePathLen &&
       path[cookiePathLen] != '/' && path[cookiePathLen] != '?') {

That would explain why the final test that I ran before I lost the account I was
using didn't work.

Comment 29

[Stephen P. Morse]

Boris, I agree completely.  Unfortunately citibank is probably too important a
customer, so if they don't cooperate with our evangelist, we will have to back
out the patch.  I'm just being a realist.

If it was just nordea, I would say leave the test in and apply pressure on them
to change.

Comment 30

[Stephen P. Morse]

cc'ing Mitch.  He probably should have been cc'ed on this earlier.

Comment 31

[Arun Ranganathan]

I'm extremely reluctant to evangelize something that looks outwardly like a
regression, no matter how emphatically one can argue the correctness of that
regression.
  
Secondly, is testing on Citibank servers an absolute necessity?  Isn't it
possible to create a local test case that sets cookies in the same way?  I'm
sure the "disabled" account will be active again shortly.  Testing a live
banking site as we QA a fix is really not wise.

There's no agenda for evangelism here -- I'm *not keen* to pull hairs with
Citibank over something that once worked.  Let's just do the fix.

Comment 32

[Boris Zbarsky [:bzbarsky]]

Arun, the issue is that "the fix" reopens a security hole that we closed up. 
I'm not exactly keen on breaking citibank either, but in this case the behavior
their site currently expects puts the user of any browser that implements it at
risk of cookies being sent to people who have no business being able to get at
them.  

It's not a correctness issue as much as a matter of how much we're willing to
compromise our users' privacy and security over the entire scope of the web
because of a single web site (citibank).

If we were sending the citibank cookies to another banking site's server, they
would be up in arms about it.  Yet that is the behavior they want us to
implement, effectively.

Comment 33

[Stephen P. Morse]

Attachment: Fix problem in comment 28. Still untested.

Comment 34

[Stephen P. Morse]

> Secondly, is testing on Citibank servers an absolute necessity?  Isn't it
> possible to create a local test case that sets cookies in the same way?  I'm
> sure the "disabled" account will be active again shortly.  Testing a live
> banking site as we QA a fix is really not wise.

No.  We never can be sure that we are duplicating the cookies exactly (there are
over 20 cookies involved) and they are used in a variety of ways.  Such testing
could be used as a starting point but the final test still has to be run on the
citibank site.  And I'm up to the final tests.

Yes the disabled account will go live again, but it was disabled a second time.
 And the person whose account it was is now less than happy to have me keep
experimenting with her account.

I agree that testing a live account is not wise.  That's why I asked if an
evangelist can get them to give us a test account.

Comment 35

[Daniel Veditz [:dveditz]]

Arun: Note that Steve found three different problems among the 20 or so cookies
they set, and that it took considerable investigation to weed out which ones
were causing the rejection. In this situation diagnosing the problem with the
site itself is critical, just as evangelism often needs to pore over the actual
web pages and stylesheets of a broken site to discover the problem, with the
huge disadvantage that the cookies can't be captured locally just once and
replayed over and over the way a web page can.

bz: Steve is proposing we reopen bug 155083 which is about *setting* cookies
with the wrong path, not reading them. The host is still checked, too. The page
could always set path to "/" and get the cookie accepted and readable by the
other page so loosening this restriction doesn't cause any privacy leaks. It
does potentially allow one page on a host to confuse another page on that same
host if the second page trusted the cookie path.

Steve: by "untested" you mean against citibank? You've tested the code in
general I assume.

Comment 36

[Daniel Veditz [:dveditz]]

Comment on attachment 100471 [details] [diff] [review]
Fix problem in comment 28.  Still untested.

sr=dveditz if you reopen bug 155083

Comment 37

[Daniel Veditz [:dveditz]]

Comment on attachment 100471 [details] [diff] [review]
Fix problem in comment 28.  Still untested.

>+      if (pathLen>cookiePathLen && path[cookiePathLen] != '/' && path[cookiePathLen] != '?') {

What about ';' and '#' -- Should you check for those too? Or wait and see if
sites break first?

Comment 38

[Mel Reyes]

How about getting something into the daily builds and letting us (users) test
Citibank.  I'll be glad to test or run a sniff/capture with some guidance.

Question 1: Why doesn't this break with other browsers like IE?  Is it open to
this security hole?  I've used all version and still haven't hit this issue.

Question 2: As crazy as this sounds I used Opera 6.05 on WinXP and it prompted 3
times because the cookie path wasn't set "legally" (I'll attach a snapshot). 
The key thing is that I refused all three, but I was able to get through, of
course it didn't remember my username etc. but it still didn't time out and it
had a very nice warning unlike IE, NS nor Moz.

Question 3: Why doesn't someone publicize this "bug" if it exists in other
browsers or major sites, I'm assuming this will eventually get into a build of
NS and since Moz and NS are such scrutinized projects, someone will notice
either the fact that we don't display an error or that the sites don't work.

Stupid Question 1: Being ignorant to the specifics of the Cookies or SSL RFC's
is there a difference in cookie path checking between standard http and secure
mode?  I'm somewhat familiar with the general cookies valiation rules but I was
just wondering, thanks.

And thanks again for the attention to this little booger.

Comment 39

[Mel Reyes]

Attachment: Opera 6.05 Illegal Cookie Path for Citibank Login

This is one of the cookie dialogs that popped up when signing onto Citibank.  I
found this feature very interesting and useful.

Comment 40

[Stephen P. Morse]

> Steve: by "untested" you mean against citibank? You've tested the code in
> general I assume.

Correct.

> What about ';' and '#' -- Should you check for those too? Or wait and see if
> sites break first?

When would these occur, what would they mean?  And can you have a semicolon? 
The syntax for the set-cookie header would have:

    set-cookie: a=b; path=xyz; domain=...

so a semicolon would end the path and could not be part of it.

> How about getting something into the daily builds and letting us (users) test
> Citibank.  I'll be glad to test or run a sniff/capture with some guidance.

By doing that you would be testing for sufficiency but not for necessity.  For
example, I'm not sure if the expire change is really necessary because there was
some confusion in the early tests that I ran.  If I could run the test myself,
then I could determine immediately which parts of the patch are really necessary.

> Why doesn't this break with other browsers like IE?  Is it open to this
> security hole?

That is correct.

> is there a difference in cookie path checking between standard http and secure
> mode? 

No.  The secure attribute on the set-cookie header means that the cookie will
not be sent back unless there is a secure (https) channel.

Comment 41

[Daniel Veditz [:dveditz]]

I meant check for '#' and ';' in the URL as you do for '?' since they are
similar special characters that can terminate the file part of the URL.

Comment 42

[Stephen P. Morse]

Attachment: add check for # and ; -- still untested

Comment 43

[Stephen P. Morse]

Comment on attachment 100673 [details] [diff] [review]
add check for # and ; -- still untested

r=dveditz (carrying forward)

Comment 44

[Stephen P. Morse]

My inclination is to check in the part of the patch dealing with the expire time
and with the getting of cookies (if file name equals path name).   However the
part of the patch that deals with the violation of the cookie spec (setting of
cookies for path not owned by server) should be left for evangelism. 

Comment 45

[Stephen P. Morse]

Looking for an r=.  Any takers?

Comment 46

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 100673 [details] [diff] [review]
add check for # and ; -- still untested

Looks OK to me.
r=mstoltz.

It's up to you about the path-check issue - I agree that we should evangelize
it.

Comment 47

[Stephen P. Morse]

OK, I checked in the other two parts of the patch (expires time, strange use of
path when sending cookies) but did not check in the setting of cookies that
violate the path test.

Will now leave this as an evangelism issue.  Please get citibank to modify their
site so that they do not set cookies with a path attribute on a path that the
current site does not have authority for.

Reassigning to evangelism.

Comment 48

[Arun Ranganathan]

OK, thanks Boris, Mitch, Dan, Stephen et al. for explaining this.  However, one
area is still unclear, and I'm looking for a security assessment.  In Comment
32, bz says that you can send cookies from one banking site to another banking
site's server:

"If we were sending the citibank cookies to another banking site's server, they
would be up in arms about it.  Yet that is the behavior they want us to
implement, effectively."

But I thought this was merely about paths within a given host or domain getting
confused, not about a completely cross-domain cookie sharing violation!  In
fact, dveditz suggests in Comment 35 that the *worst that can happen* is that
you can potentially confuse paths within a given host:

"It does potentially allow one page on a host to confuse another page on that
same host if the second page trusted the cookie path."

Now can someone scare the pants off me with a scenario that brings it all home?
Don't misunderstand me -- I'm *against* RFC violations (as morse mentions in
Comment 20 and Comment 21) but I'd really like a solid story if I'm going to
phone a major international bank and tell them that something's not absolutely
kosher.  The good news is -- if evangelism becomes an absolute necessity, then
the story going forward is really that no Netscape release currently in
circulation (N7 RTM, even Blackbird) will break, but that future browsers won't
be as tolerant of this flaw for their user-base's sake.

Comment 49

[Boris Zbarsky [:bzbarsky]]

Arun, there's this unspoken assumption that all paths on a single hostname are
friendly and care about each other.  This is simply not the case in the real
world; eg with any university's website.  I'm not saying that we'd be sending
one bank's cookies to another.  I'm saying we would be doing the equivalent.

I'm afraid I can't scare the heck out of you.  It's not a "lose your life or
freedom" type thing; just a privacy/security violation that could lead to acute
embarrasment (and people getting access to information that they should not have
access to; the implications of this last could, of course, be anything).

Comment 50

[Arun Ranganathan]

In Comment 49 , bz raises the classic "GeoCities" scenario which is that
different paths on a consistent host/domain don't necessarily mean that every
path is friendly to every other path.  I acknowledge this, and think Mel Reyes
attachment in Comment 39 is a happy compromise.  I'm not scared out of my wits;
I think I'm going to seek advice on whether to evangelize or not from many
different sources.

Comment 51

[Boris Zbarsky [:bzbarsky]]

I agree that putting control over this in the hands of the user may be a decent
compromise. Note that I specifically avoided mentioning geocities because imo
people tend to blow off geocities as "who cares about them".  The problem is,
many other much more serious sites are also affected by the same scenario...

Comment 52

[Stephen P. Morse]

Patch for bug 155083 has been backed out.  This site should now be working

Comment 53

[lohphat]

win32 trunkbuild 2002100208 still has problem -- waiting for new build.

Comment 54

[lohphat]

win32 2002100308 build now working again

Comment 55

[Vadim Kutsyy]

Citibank finaly seems to implement new website (it was on/off for last few
month).  On new website, timeout is not a problem, but information (i.e. list of
charges, payments etc.) doesn't show up, and it doesn't display any errors. 
Balances shows up just fine.

Comment 56

[lohphat]

So far WFM with 2002100710 on 2K-pro

Comment 57

[Vadim Kutsyy]

Just tested new citi website with 2002100804 build.  Works fine for me.  Once I
got a mozilla crush, but I am not sure it was related (Talkback TB12256689W).  I
had another 8 tabs open. 

As a site note, latest builds are fast, but citibank website is significantly
slower than IE 6.1.

Comment 58

[J.J. Enser]

confirming. I've tested their new site this weekend, checking balances, making
payments & transfers, and no problems encountered using NS7 on MacOS X. No
timeout when using the browser back/forward buttons either!

Congrats to all for the excellent evangelism work. It worked!

Comment 59

[Stephen P. Morse]

The site wasn't evangelized, at least not that I know of.  Instead we relaxed
our cookie checking, even though we were doing what the standard called for, and
backed out the patch in bug 155083.  That's what fixed things.

So much for standards. :-(

Comment 60

[Rahul Siddharthan]

I'd like to report that the citibank site works for me with the latest nightly
mozilla, and with mozilla 1.1 and earlier, but not with 1.2alpha.  Thanks for
getting it working, and I'm alarmed at the nature of the comments about
Citibank's cookie handling etc and have submitted a consumer feedback form to
Citibank pointing them to this page and expressing concern about their security.

Comment 61

[Michael Buckland]

Sent mail to Arun asking could you please take a look at bug and update
accordingly.  Said it looks to me like you might be able to close the bug, but
if you are not able to do that could you please target a Mozilla milestone? 
This bug is currently untargeted.

Comment 62

[Arun Ranganathan]

Now technically, this bug can be closed because there's no longer an evangelism
issue here.  Morse backed the patch out; without the patch, things should work
as they were before.  
I can close this bug, but I think that I ought to first seek dveditz's approval,
since he was one of forces behind the backing out of the patch.  Dan, can we lay
this issue to rest?

Comment 63

[Michael Buckland]

Arun, assuming you can get dvedtiz approval, please close this.

Comment 64

[dan]

Arun,
If you are referring to me, I would say put it to rest.  I have not had a 
problem with CitiBank for a while now.

Thanks,
Dan

Comment 65

[chris hofmann]

closing

Comment 66

[Paul Wyskoczka]

verifying per comments

Comment 67

[Nick Bebout]

Moving to new component

Comment 68

[lohphat]

Why moving? It's fixed.  2003061108 win32 WFM

Comment 69

[Matthias Versen [:Matti]]

You can remove only a component in bugzilla if there is no bug in the component..