Opened 4 years ago

Closed 3 years ago

#19778 closed Bug (wontfix)

csrf middleware report BAD_REFERER when HTTP_HOST contains port

Reported by: jens.tinfors@… Owned by:
Component: CSRF Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

For a given request with the HTTP_HOST header set to 'www.example.com:443' and the HTTP_REFERER set to 'https://www.example.com/somepage' the same_origin check will report REASON_BAD_REFERER due to fact that HOST has port and REFERER does not. Perhaps, as in my case, a less competent firewall has tampered with the request on its way to the server.

Here's a failing test (that I added to tests/regressiontests/csrf_tests/tests.py):

   def test_https_good_referer_with_port(self):
        req = self._get_POST_request_with_token()
        req._is_secure_override = True
        req.META['HTTP_HOST'] = 'www.example.com:443'
        req.META['HTTP_REFERER'] = 'https://www.example.com/somepage'
        req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) 
        self.assertEqual(None, req2)

I know the port is a valid part of the same_origin check but I'm thinking that if it's missing from REFERER, the scheme is https and HTTP_HOST uses the default port, maybe we can skip the port from the same_origin check.

What do you think?

Incidentally, if I set a port number on the HTTP_REFERER the test still fails:

   def test_https_good_referer_with_port(self):
        req = self._get_POST_request_with_token()
        req._is_secure_override = True
        req.META['HTTP_HOST'] = 'www.example.com:443'
        req.META['HTTP_REFERER'] = 'https://www.example.com/somepage:443'
        req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) 
        self.assertEqual(None, req2)

kind regards /jens

Change History (6)

comment:1 Changed 4 years ago by Simon Charette

Version: 1.4master

comment:2 Changed 4 years ago by Claude Paroz

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

comment:3 Changed 4 years ago by Erik Romijn

Keywords: sprint2013 added
Owner: changed from nobody to Erik Romijn
Status: newassigned

comment:4 Changed 4 years ago by Erik Romijn

Keywords: sprint2013 removed
Owner: Erik Romijn deleted
Status: assignednew

comment:5 Changed 3 years ago by Bouke Haarsma

Type: Cleanup/optimizationBug

From the HTTP/1.1 RFC 2616:

A "host" without any trailing port information implies the default port for the service requested (e.g., "80" for an HTTP URL). For example, a request on the origin server for <http://www.w3.org/pub/WWW/> would properly include:

The tests you've added also fail in the current master, and this should be categorized as a bug.

charettes, your patch is quite out-dated and cannot be applied on master. Also there's some styling issues with the code (correct use of spaces in dicts). If you could fix those issues and squash the commits into a single commit, this patch could be moved forwards.

comment:6 Changed 3 years ago by Bouke Haarsma

Resolution: wontfix
Status: newclosed

From the submitter (comment on GitHub)

Well, It's been almost a year since I submitted this pull request.
I say in the comment above that you should probably disregard it.

A year old bug!? If a bug at all!? "The tests fail in master", I shall hope so, it's a year old.

Let's close this one and move on shall we!

Note: See TracTickets for help on using tickets.
Back to Top