No referrer sent for meta refresh

ASSIGNED
Assigned to

Status

()

Core
Document Navigation
ASSIGNED
13 years ago
9 months ago

People

(Reporter: dveditz, Assigned: dveditz)

Tracking

({fixed-aviary1.0, fixed1.7.5})

Trunk
fixed-aviary1.0, fixed1.7.5
Points:
---
Dependency tree / graph
Bug Flags:
blocking-aviary1.0 +
blocking1.8b -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

13 years ago
No referrer is sent when a document is loaded as a result of a <META
HTTP-EQUIV="refresh">. There are two candidates for a referrer, the original
referrer, or the document containing the META tag.

Docshell already makes a distinction based on the refresh timer, short ones are
considered a "redirect" and longer ones are a "refresh", although it would seem
to make more sense to compare the URIs instead. I propose

redirects: use the original document's referrer, like HTTP redirects

refreshes: use the document being refreshed, i.e. it refers to itself.

Comment 1

13 years ago
What does IE do?
(Assignee)

Updated

13 years ago
Blocks: 61660
(Assignee)

Comment 2

13 years ago
IE doesn't appear to send a referrer, or at least document.referrer doesn't get
set. But that doesn't make it right, and I need a referrer to fix 264560.

Comment 3

13 years ago
do you really need a referrer?  or do you just need to set that special docshell
referrer property on the channel?
(Assignee)

Comment 4

13 years ago
Yes, that's all I need. The part that sets that currently gets that from the
referrer argument that's also used to set the referrer on the channel. And
currently it gets nothing because nsDocShell::Notify() creates a
nsDocShellLoadInfo with a null referrer.

How can sending a referrer in this case be wrong?
(Assignee)

Comment 5

13 years ago
pardon me, nsRefreshTimer::Notify()

Comment 6

13 years ago
> How can sending a referrer in this case be wrong?

I simply worry about changing the behavior of the webplatform, especially when
it deviates from established behavior (by us and IE).

Though the Referer header is technically optional (according to RFC 2616), there
are plenty of sites that break if it is not sent, or if it is not the expected
value.

Sure, you are talking about sending it when it is currently not sent, but might
that not screw up web tracking software that doesn't want to count meta
refreshes as link clicks?

I'm just suggesting that there might be some really wanky web dependencies on
this stuff, which may make this a risky change :-/
(Assignee)

Comment 7

13 years ago
Created attachment 163915 [details] [diff] [review]
send referrer for meta refresh
(Assignee)

Updated

13 years ago
Attachment #163915 - Flags: superreview?(jst)
Attachment #163915 - Flags: review?(darin)
Darin's right.  If this is being proposed for branches, I would be against
sending a referrer in the actual HTTP request -- that's a pretty risky change. 
If this is just being proposed for trunk, it may be ok, but needs a lot of testing.
Anyone got any clues on how to do this cleanly w/o passing the referrer to the
http request etc in the docshell code? More arguments to LoadURI etc?
Can't we just set the referrer property in the nsIProperties directly, like
suggested in comment 4?
I guess the problem is that the refresh code that would set it doesn't have the
channel, so no nsIProperties to set it on.
Ah, I see. Then the answer is to extend loadinfo to indicate whether the
referrer should actually be set or not, perhaps?  At least in the short run...
Created attachment 164218 [details] [diff] [review]
Maybe something like this then?

This adds an internalReferrer to nsIDocShellLoadInfo, but I had to move the
code that sets it on the channel, that will now happen later, if that's not ok
then we'll need to push this info further in...
Yeah, that looks ok from the docshell end.  I suspect it's fine from necko too,
but I don't recall the exact details of the nsIProperties stuff.

Longer-term, perhaps we should think about how to do this more cleanly, but for
branch that looks fine.
Created attachment 164229 [details] [diff] [review]
Cleaner, I think...

This adds a boolean noReferrer to nsIDocShellLoadInfo and makes the code use
that to prevent the referrer from being sent to the server in the refresh case.
This changes some arguments to InternalLoad() and DoURILoad() in the docshell
code, but it's got the benefit of not changing when the internal referrer is
set on the channel (through nsIProperties).

Updated

13 years ago
Attachment #164229 - Flags: superreview?(darin)
Attachment #164229 - Flags: review?(dveditz)
Oh, and with the latest diff I'll need to rev nsIDocShell's IID too, done locally...
(Assignee)

Comment 17

13 years ago
Is changing nsIDocShellLoadInfo interface on the branch the right thing? In some
ways that seems like a worse idea than sending an extra referrer (on the other
hand it's probably not that useful an interface for external code).

By moving the property setting up to LoadURI you miss Reload and
LoadHistoryEntry, but I'm not sure how bad that might be. You could catch them
all by doing the seeting in LoadInternal, by adding yet another owner-ish type
argument :-)

Oh, wait -- nsWebShell::OnLinkClickSync() calls InternalLoad directly, bypassing
LoadURI() entirely, so the property will never be set and we'll be back to bug
265135
(Assignee)

Comment 18

13 years ago
Previous comments were directed at the "Maybe something like this" patch, the
"Cleaner, I think..." patch addresses exactly those concerns. Looks good from a
quick scan.
Flags: blocking-aviary1.0+

Comment 19

13 years ago
> Oh, and with the latest diff I'll need to rev nsIDocShell's IID too, done
locally...

best not to change the IID of nsIDocShell.  we have embedders (Eclipse) who are
using nsIDocShell::loadStream.  fortunately, i think this change can be made
without rev'ing the IID of nsIDocShell since the interface isn't changing in a
binary incompatible way (assuming existing callers of InternalLoad all pass 0x1
for true).

Comment 20

13 years ago
Comment on attachment 164229 [details] [diff] [review]
Cleaner, I think...

>+  const long INTERNAL_LOAD_NO_REFERRER   = 0x2;
...
>+    /** True if the no referrer should be sent, even if it's available
>+     */
>+    attribute boolean noReferrer;

I'd really like a better name for this attribute.  noReferrer sounds
odd.  "If you don't want a referrer, then why are you setting the
referrer attribute?"

How about if we call it "sendReferrer" with a default value of true?
I like "sendReferrer" because it seems more consistent with what 
actually happens in the implementation.

sr=darin with that change.
Attachment #164229 - Flags: superreview?(darin) → superreview+

Comment 21

13 years ago
But I guess you'd still want the sense of the InternalLoad flag to be something
like: INTERNAL_LOAD_DONT_SEND_REFERRER
(Assignee)

Comment 22

13 years ago
Comment on attachment 164229 [details] [diff] [review]
Cleaner, I think...

Just to be clear, this patch does not fix *this* bug (send a referrer), it
fixes bug 264560. I filed this bug as one way to fix bug 264560, but even apart
from that sending the referrer in the refresh case seems like the right thing
to do. Just maybe not now for Firefox 1.0 since that change seems to scare
smart people.

r=dveditz (but please leave this bug open and close 264560 when checked in)

I'm not keen on the "noReferrer" name, btw, nor the hard-to-read "!(x & FLAG)"

>+    rv = DoURILoad(aURI, aReferrer, !(aFlags & INTERNAL_LOAD_NO_REFERRER),

But it's correct and works.

Also may want a #define for 0 to make the following clearer:

>     rv = InternalLoad(uri,
>                       referrerURI,
>                       nsnull,            // No owner
>-                      PR_FALSE,          // Do not inherit owner from document (security-critical!)
>+                      0,                 // Do not inherit owner from document (security-critical!)
Attachment #164229 - Flags: superreview?(darin)
Attachment #164229 - Flags: superreview+
Attachment #164229 - Flags: review?(dveditz)
Attachment #164229 - Flags: review+
Comment on attachment 164229 [details] [diff] [review]
Cleaner, I think...

a=ben@mozilla.org
Attachment #164229 - Flags: approval-aviary+
Created attachment 164338 [details] [diff] [review]
Final diff that was checked in.
Fix checked in. Leaving bug open, but adding fixed-aviary1.0 to get this off the
1.0 blocker radar.
Keywords: fixed-aviary1.0
(In reply to comment #21)
> But I guess you'd still want the sense of the InternalLoad flag to be something
> like: INTERNAL_LOAD_DONT_SEND_REFERRER

Duh, forgot to do this before checking in, I made this change on the branch
already tho.

Comment 27

13 years ago
Ack! I think this caused some pretty serious problems with today's Thunderbird
0.9 release. cc'ing myself. In the bits from this morning that we were going to
release, Thunderbird has all sorts of weird wallet problems. 

Between yesterday and today, Thunderbird can no longer log onto mail servers.
Wallet fails to bring up UI for prompting for passwords instead the attempt to
bring up the UI fails (I think) and we end up sending an empty password for
everything.

This effects trying to read IMAP, POP and news that requires auth dialogs.

This was the only checkin for Firefox between yesterday and today that would
have effected Thunderbird as well which is why I'm starting off by pointing the
finger here even if that may not be the case. 
> This was the only checkin for Firefox between yesterday and today that would
> have effected Thunderbird as well

No, it was not.  You're likely seeing fallout from bug 267367...  that touched
docshell code in a much more fundamental way than this bug did, and would have
had exactly the effects you describe.

Comment 29

13 years ago
Comment on attachment 164338 [details] [diff] [review]
Final diff that was checked in.

this version of the patch still uses INTERNAL_LOAD_FLAGS_NO_REFERRER.  was that
intentional?
(In reply to comment #29)
> (From update of attachment 164338 [details] [diff] [review])
> this version of the patch still uses INTERNAL_LOAD_FLAGS_NO_REFERRER.  was that
> intentional?
> 

No, see comment 26.

Comment 31

13 years ago
Can sending a referer at least be a togglable option? Many websites (inlcuding
my own) use the meta refresh to link people to other pages without that other
page knowing where the visitor came from.

IIRC Opera 7.2+ is the only browser that does send a referer on a meta refresh.
The rest doesn't.

Comment 32

13 years ago
Comment on attachment 164229 [details] [diff] [review]
Cleaner, I think...

sr=darin for the version that was checked in.
Attachment #164229 - Flags: superreview?(darin) → superreview+

Updated

13 years ago
Attachment #163915 - Flags: superreview?(jst)
Attachment #163915 - Flags: review?(darin)
(Assignee)

Comment 33

13 years ago
Comment on attachment 163915 [details] [diff] [review]
send referrer for meta refresh

No, I really do want reviews on this, we should do this at least on the trunk.
I'm happy to hear Opera supports this, it bolsters my contention that it won't
break anything.
Attachment #163915 - Flags: superreview?(jst)
Attachment #163915 - Flags: review?(darin)

Comment 34

13 years ago
Comment on attachment 163915 [details] [diff] [review]
send referrer for meta refresh

dveditz: ah, right... ok.  so, i'm willing to give this a try on the trunk. 
does a new patch need to be cut considering the recent changes?
Comment on attachment 163915 [details] [diff] [review]
send referrer for meta refresh

sr=jst for this change. The other change in this bug didn't land on the trunk
yet (should it?), so this would apply against the trunk, me thinks.
Attachment #163915 - Flags: superreview?(jst) → superreview+

Comment 36

13 years ago
Comment on attachment 163915 [details] [diff] [review]
send referrer for meta refresh

sure, let's go with this on the trunk.
Attachment #163915 - Flags: review?(darin) → review+
Note that this bug already has one comment about a site it _will_ break...  so
"won't break anything" is a rather tenuous contention here.

The real question is whether we think it will break few enough things that we're
ok with breaking them for the theoretical purity of sending referrer on meta
refreshes.

Comment 38

13 years ago
hmm, yeah..  i had overlooked comment #31 when i marked r+ on the patch. 
perhaps we should just take the aviary patch then.
(Assignee)

Comment 39

13 years ago
(In reply to comment #31)
> Can sending a referer at least be a togglable option? Many websites (inlcuding
> my own) use the meta refresh to link people to other pages without that other
> page knowing where the visitor came from.

What's your site and some of the others? I found a lot of complaints about not
sending the referrer in this case, but few using it for user-helpful reasons.
The fact that newer Opera versions started sending the referrer in this case may
mean they came to the same conclusion.

I guess we should run this by WHAT-WG to see if we can get a common approach.
Status: NEW → ASSIGNED

Comment 40

13 years ago
Whatever went in aviary needs to go in 1.7 for gecko compatibility....
(Assignee)

Updated

13 years ago
Blocks: 272764
(Assignee)

Comment 41

13 years ago
Checked into 1.7 branch
Keywords: fixed1.7.5

Comment 42

13 years ago
What about using "network.http.sendRefererHeader" for meta redirects/refreshes?
Will that have any effect on this?

Updated

13 years ago
Flags: blocking1.8a6?

Comment 43

13 years ago
Does this need to land on the trunk? If so, who can do that? 
(Assignee)

Comment 44

13 years ago
yes, and me, today
(Assignee)

Comment 45

13 years ago
aviary fix checked in to trunk. Leaving bug open because the patch really fixes
bug 264560, not the summary item (objection in comment 31 noted).
(Assignee)

Updated

13 years ago
No longer blocks: 272764

Updated

13 years ago
Flags: blocking1.8a6?

Updated

13 years ago
Flags: blocking1.8b?

Updated

13 years ago
Flags: blocking1.8b? → blocking1.8b-

Comment 46

12 years ago
(In reply to comment #39)
> What's your site and some of the others? I found a lot of complaints about not
> sending the referrer in this case, but few using it for user-helpful reasons.

GMail uses this technique to hide the user's authentication credentials when clicking on a hyperlink in an email.

It would be a bad idea to change this feature. Currently, this technique is one of the few feasible ways to prevent sending of the Referer header and thus protect the user's privacy/security.

Comment 47

12 years ago
*** Bug 322216 has been marked as a duplicate of this bug. ***

Comment 48

12 years ago
I'm not sure what's going on after it has been a known issue for a year and still not fixed in Firefox 1.5. Opera does it right -- IE6 and Firefox do it the way that actually helps the "bad" guys.


A real world problem: Stealing Bandwidth

This issue can be exploited to steal bandwidth from a server and its admin won't have ANY chance to see where the hot-linked traffic is coming from. (Let me know if you need me to elaborate.)


Incosistency: HTTP Redirect (301/302) does send referers. Meta Refresh redirect does not. (Only Microsoft could have created something as inconsistent as this).

Comment 49

12 years ago
(In reply to comment #48)
> A real world problem: Stealing Bandwidth
> 
> This issue can be exploited to steal bandwidth from a server and its admin
> won't have ANY chance to see where the hot-linked traffic is coming from. (Let
> me know if you need me to elaborate.)

Who cares where it's coming from, so long as you can block it? Configure your server to send an error response if the expected Referer header is not present. Problem solved.

On the other hand, if there is no way to prevent sending of a Referer header, it becomes very difficult to protect a user's privacy/security when linking from a private resource. For example, GMail relies on the meta refresh technique to hide the URL of the email containing the link. Without the meta refresh technique the target of the hyperlink gets access to the GMail URL, which contains session information (see the auth= query parameter).

Changing the behaviour of meta refresh has bad consequences for access control in WWW applications. A lot of security vulnerabilities will be created. These vulnerabilities will also be very hard to close.
Ian, does WHATWG plan to specify this?
There's no "http-equiv" attribute in HTML5 at the moment. Does that answer your question? :-)

Comment 52

12 years ago
(In reply to comment #49)
> Who cares where it's coming from, so long as you can block it? Configure your
> server to send an error response if the expected Referer header is not present. 
> Problem solved.

By denying traffic with an empty referer you create only a new problem. There are many user agents that do not send referers (in some of them this behaviour is optional, in some it is hard-coded). Then there are those "Internet Security Software Solutions" (firewalls surrounded by other tools) that clear the referer in order "to protect your privacy". All these people would be constantly getting errors when trying to download the "protected" file. The result would be endless hundreds of emails to webmasters saying that their downloads are broken. The bottom line, no, you cannot deny traffic with an empty referer (so your comment "Problem solved" is in fact incorrect). 



> On the other hand, if there is no way to prevent sending of a Referer header,
> it becomes very difficult to protect a user's privacy/security when linking
> from a private resource.

As we all know, there is no such thing as privacy on the internet. Let's not be naive here. However, if you need to censor the referer, copy the link manually to the adress box and press Enter. Problem solved.



> For example, GMail relies on the meta refresh technique to hide the URL of
> the email containing the link.

I seriously doubt that you heard this from a Google engineer. Why? Because he might be fired for not securing their system properly. Why? 
1) Opera does send referers on meta refresh redirection. So about 7% of their visitors would be insecure (yes, according to our stats, 7% of visitors use Opera). 
2) Their system (Gmail) must be secured not in this lame way, but in a proper and professional way (and I believe it is) so that it protects Opera users too. When someone gets a referer leading to your Gmail Inbox, he can't be allowed to access that URL without logging in. It is quite easy to do (the session ID may be connected with a particular IP/cookie, etc.) Ask the Yahoo Mail engineers how they did it -- they know better than I do.
The bottom line, this argument is invalid too.



> Changing the behaviour of meta refresh has bad consequences for access control
> in WWW applications. A lot of security vulnerabilities will be created.

Name some. (I already named the consequences of not sending the referer). Again, Opera has been doing this right for a long long time and nothing has broken. Your third argument is again invalid.

Comment 53

12 years ago
To summarize:

If anyone designed (the security of) his web pages to count on the fact that browsers do not send referers on meta refresh redirection, then it is solely HIS FAULT BECAUSE THIS BEHAVIOUR IS NOT GUARANTEED TO WORK IN ALL BROWSERS (Opera behaves diferrently). It is solely his failure and he will face consequences for it if the default behaviour is changed.


Sorry, but if Firefox does not fix this soon, I hope that Google will buy Opera and that it will become the most used browser pushing Firefox into oblivion. Not sending a referer in general is harmful to the health of the entire web infrastructure. (Only a bunch of malevolent hackers could think otherwise.)
From that bug it seems that Gecko is supposed to send a Referer header for meta refresh. However, with Firefox 2.0.0.3 I see that it doesn't. This has some security relevance: http://kuza55.blogspot.com/2007/03/non-persistent-untraceable-xss-attacks.html

As to concerns about credentials leaking through referrer headers - all cases that I've seen use a neutral dereferer page that doesn't have any credentials in the URL. The target page should only get the URL of this dereferer page.
Oh, I see - the correct referrer is set internally but not sent to the web page. I think this needs to be fixed.
(Assignee)

Updated

11 years ago
Whiteboard: [sg:want P2]

Comment 56

10 years ago
What is going on here? This is a security-related bug. Why hasn't it been fixed already? Why does it take years? 

YOU ARE ALLOWING MALEVOLENT PEOPLE TO CLEAR THE REFERRERS TO CONCEAL THEIR XSS ATTACKS, HOT LINKING, AND OTHER MALEVOLENT PRACTICES WITHOUT BEING TRACEBLE BY THE VICTIMS. DO SOMETHING ABOUT IT ALREADY!
Actually there are pages (like GMail) that now depend on this to avoid leaking private information that is stored in their URLs. So arguably this shouldn't be fixed.

Note that if you believe not sending referrers is a security problem, it would be helpful to provide feedback to the HTMLWG or WHATWG where I just added rel=noreferrer to explicitly prevent a link from including the referrer header.
QA Contact: adamlock → docshell

Updated

6 years ago
Whiteboard: [sg:want P2]
You need to log in before you can comment on or make changes to this bug.