Open Bug 266554 Opened 20 years ago Updated 2 years ago

No referrer sent for meta refresh

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

People

(Reporter: dveditz, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: DevAdvocacy, fixed-aviary1.0, fixed1.7.5)

Attachments

(5 files)

send referrer for meta refresh
2.51 KB, patch
darin.moz
: review+
jst
: superreview+
Details | Diff | Splinter Review
Maybe something like this then?
9.87 KB, patch
Details | Diff | Splinter Review
Cleaner, I think...
28.50 KB, patch
dveditz
: review+
darin.moz
: superreview+
bugs
: approval-aviary+
Details | Diff | Splinter Review
Final diff that was checked in.
28.73 KB, patch
Details | Diff | Splinter Review
P1 Set the current document's URL as the referer when navigating via <meta http-equiv="refresh">. r=bz
1.42 KB, patch
Details | Diff | Splinter Review 
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.
What does IE do?
Blocks: 61660
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.
do you really need a referrer?  or do you just need to set that special docshell
referrer property on the channel?
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?
pardon me, nsRefreshTimer::Notify()
> 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 :-/
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...
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.
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).
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...
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
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+
> 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 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+
But I guess you'd still want the sense of the InternalLoad flag to be something
like: INTERNAL_LOAD_DONT_SEND_REFERRER
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+
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.
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 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.

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 on attachment 164229 [details] [diff] [review]
Cleaner, I think...

sr=darin for the version that was checked in.
Attachment #164229 - Flags: superreview?(darin) → superreview+
Attachment #163915 - Flags: superreview?(jst)
Attachment #163915 - Flags: review?(darin)
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 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 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.
hmm, yeah..  i had overlooked comment #31 when i marked r+ on the patch. 
perhaps we should just take the aviary patch then.
(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
Whatever went in aviary needs to go in 1.7 for gecko compatibility....
Blocks: 272764
Checked into 1.7 branch
Keywords: fixed1.7.5
What about using "network.http.sendRefererHeader" for meta redirects/refreshes?
Will that have any effect on this?
Flags: blocking1.8a6?
Does this need to land on the trunk? If so, who can do that? 
yes, and me, today
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).
No longer blocks: 272764
Flags: blocking1.8a6?
Flags: blocking1.8b?
Flags: blocking1.8b? → blocking1.8b-
(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.
*** Bug 322216 has been marked as a duplicate of this bug. ***
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).

(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? :-)
(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.

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.
Whiteboard: [sg:want P2]
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
Whiteboard: [sg:want P2]
See bug 1454022 for a recent interop issues due to this.  Now that we have referrer-policy and CSP, can we expose this?

Boris, Christoph, any opinions?
Flags: needinfo?(ckerschb)
Flags: needinfo?(bzbarsky)
What is the actual behavior being proposed?  What does the spec say?  What do other browsers implement?
Flags: needinfo?(bzbarsky)
The spec (as far as I traced it last night) says to treat meta refresh as a navigation with replacement true.

Bug 1454022 suggests both chrome and safari set a referrer on meta refresh.

I guess I was wondering if there was some historical or gecko-specific policy driving our difference.

Any way I can look at the spec and other browsers more next week.
Flags: needinfo?(ckerschb)
I made a quick demo that shows the referer after an iframe does a meta-refresh:

  https://meta-refresh-referer.glitch.me/

In the various browsers the iframe shows:

Firefox: blank
Chrome, Safari, Edge: "https://meta-refresh-referer.glitch.me/"

So I think we are the only browser not implementing referer on refresh.

I want to make this test check referrer-policy, though.
Sorry, slightly different:

Chrome: "https://meta-refresh-referer.glitch.me/frame.html"
Edge: "https://meta-refresh-referer.glitch.me/"

So it seems edge is only providing the origin.
Safari shows the full referer URL as well: "https://meta-refresh-referer.glitch.me/frame.html"
So edge is actually providing the referer header passed to the navigation for the document that meta-refreshes.  Its trying to simulate referer header processing as done for a normal redirect.  (The referer header is copied instead of using the redirecting URL as the referer.)

Internally firefox has similar logic:

https://searchfox.org/mozilla-central/rev/fcd5cb3515d0e06592bba42e378f1b9518497e3d/docshell/base/nsDocShell.cpp#6413-6419

Firefox just doesn't send this referer.

Chrome and safari seem to send the URL of the document with the <meta refresh> as the referer instead.

I *think* the spec matches chrome/safari behavior because it does not explicitly set the fetch API's request referer.  So fetch request referer will be "client" and extract the current URL.  Its unclear if this is intended, though.  I'll write a spec issue.
I'm going to steal this for a bit.  Not sure if I'll be able to get it over the finish line or not, though.
Assignee: dveditz → bkelly
This makes us behave like chrome where we use the current document's URL and also respect referrer policy.  I believe it also matches safari, but they don't have referrer policy implemented.
Let's see what breaks:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a03c80b035beb238437dfb22e6a0b622679306af

Note, this also only changes navigation via meta-refresh.  I didn't touch the case where we just refresh without navigating yet.  I want to test the other browsers.
See Also: → 1479017
Assignee: ben → nobody
Status: ASSIGNED → NEW
See Also: → 968861
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: