33762 - Cache security problems.

Status: VERIFIED WORKSFORME

(Core :: Networking: Cache, defect, P3)

Description

[Doug Turner (:dougt)]

When a channel is cached, the security info (if one exists) is not saved.  This 
prevents proper security checks when a page is reloaded.

Comment 1

[Doug Turner (:dougt)]

john, if you see lock icon problems, try disabling the mem cache.  (it is in the 
preferences dialog under Debug).  

Comment 2

[Doug Turner (:dougt)]

more information - generalizing bug.

I have a docloader observer which tracks page loads so that I can
mantain the state of the lock icon.  Currently is is very broken.  This
is a recent problem as it was working on Thrusday of last week.

  1.  There are no OnStart docloader observer notifications for anything
pulled from the cache.  For example, if a page has a image on it that you have 
seen before on a prior page, there is not docloader observation

  2.  Security Information is lost when items are cached.  Currently, it just 
returns nsnull.  I think that this was my doing when I added this to the 
nsIChannel interface.

  3.  WHen I set the loadgroup to (nsIChannel::INHIBIT_PERSISTENT_CACHING && 
nsIChannel::FORCE_RELOAD) from the onStartDocumentLoad, the page never ends.  
Other combinations still use the cache.  

  4.  When I make nsHTTPChannel::CheckCache() to just return NS_OK, everything 
works.

  To check out the problem:

1. goto https://junruh/tests.html
2. click on test "5.) Lock is mixed at this site."
3. after page is done loading, type "https://junruh/tests.html" in the url bar 
or click the back arrow.
4. note the lock icon.

Comment 3

[Doug Turner (:dougt)]

the above example is due to the security info object not being cached.


Another example is:

1. goto https://junruh/tests.html
2. click on test "5.) Lock is mixed at this site."
3. note the broken icon
4. click reload.

The page will now have a lock!!  This is because there was no notification for 
the the gif image on the page.

Comment 4

[ruslan]

I changed it to not cache secure channels for now and will make it cache it into 
memory soon.

Comment 5

[Warren Harris]

Moving to M17 which is now considered part of beta2.

Comment 6

[ruslan]

It caches the security info into the memory cache now.

Comment 7

[John Unruh]

Reopening. See dougt's comments 1. goto https://junruh/tests.html
2. click on test "5.) Lock is mixed at this site."
3. note the broken icon
4. click reload.

The page will now have a lock!!  This is because there was no notification for 
the the gif image on the page.

Comment 8

[Doug Turner (:dougt)]

mscott was working on this.  

Comment 9

[leger]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 10

[Scott MacGregor]

Is the argument here that images coming out of the cache don't get inserted
separately into the load group so the security code doesn't detect that we
loaded an image?

This sounds more like someone dealing with the cache should be involved with
more than me if my understanding of this bug is right.

Comment 11

[Scott MacGregor]

Okay the reason you don't get load notifications on the image is because we
don't try to load the image again when you reload the page. It's getting cached
somewhere. Is there a image cache that's separate from the necko memory cache or
are images stored here as well?

I'd really like to load balance this bug. I have 14 nsbeta2+ bugs right now. If
anyone feels this is something they may know about please speak up. 

Comment 12

[Doug Turner (:dougt)]

imglib has its own cache.  What happens is the necko initally download the 
file and hands it off to the image library.  It decompresses it and caches that 
image.  When layout resizes or reloads, it asks the imglib for the gif.  Imglib 
(silently to the webprogress interface) hands back the image.  It is never put 
into necko's load group, hence the secure UI never gets a notification.

We could:

1. Remove the image cache.  pnunn was thinking about this, but it is some work.
2. Make the webprogress a listener on the image lib callback.  I looked at doing 
this.  it should not be too hard.
3. Make layout call the webprogress notifications.


I spoke to jar about this a while back.  We really did not think that this is a 
beta2 bug.  I am going to remove the beta2 PDT+ status.   PDT talk to jar about 
this one.  There is not an exploit if of this security cache bug.  It basically 
is a bad UI which doesn't cause any harm.  There are other security ui bugs that 
we should be looking at.  

Comment 13

[Jim Roskind]

I think the issue is that this bug is only a problem if the site a) uses SSL; b)
puts non-secure (non-HTTPS) image links on their page.

If they design their site this way, then some browsers (4.x?) would show the
page as non-secure.  The point is that the images could be spoofed (via DNS games).

A well designed site will be consistent in their use of SSL, and hence this is
not a problem.

I'd argue that this is simply not a real-world security problem.  Sites that
have problems with this issue have already hurt their security by their own actions.

Note that the "outermost" page must be SSL (and can't be confused by this
issue), and hence it should not be possible for an attacker to merge content
from an attack site with a real site.

My vote is also for a beta2 minus.

Comment 14

[Scott MacGregor]

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

Comment 15

[Jay Patel [:jay]]

Putting on [nsbeta2-] radar. Not critical to beta2. 

Comment 16

[Scott MacGregor]

Do you guys think this security issue is severe enough that we need to fix it
for beta3 (which means this release)?

Comment 17

[Jim Roskind]

I see absolutely no urgency in repairing this bug.  I believe we have many very
urgent bugs on our plate.

IMO, this should come very late in your list of priorities.  I would not
complain if we shipped with the current status quo.

Comment 18

[Scott MacGregor]

Thanks....in that case --> Future. 

Comment 19

[Zach Lipton [:zach]]

rtm keyword. This could be critical if someone tried to check the security 
and it responded that a site was secure when it infact wasn't

Comment 20

[selmer (gone)]

rtm-.

Zach, is that case even possible?  The desription of the bug would seem to
indicate that you'd only get the opposite case of it saying a secure site wasn't.

Comment 21

[gordon]

Hey Scott, we have a completely new cache now.  Is this bug still valid?

Comment 22

[Boris 'pi' Piwinger]

Over one year nobody said that the bug is still there which was asked in the
previous comment. So I mark this as WFM.

pi

Comment 23

[Tom Everingham]

verified