Opened 3 years ago

Closed 21 months 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 3 years ago by charettes

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.4 to master

comment:2 Changed 3 years ago by claudep

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

comment:3 Changed 3 years ago by erikr

  • Keywords sprint2013 added
  • Owner changed from nobody to erikr
  • Status changed from new to assigned

comment:4 Changed 3 years ago by erikr

  • Keywords sprint2013 removed
  • Owner erikr deleted
  • Status changed from assigned to new

comment:5 Changed 21 months ago by bouke

  • Type changed from Cleanup/optimization to Bug

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 21 months ago by bouke

  • Resolution set to wontfix
  • Status changed from new to closed

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