diff --git a/weboob/tools/browser2/browser.py b/weboob/tools/browser2/browser.py index 019d45ab..57402be9 100644 --- a/weboob/tools/browser2/browser.py +++ b/weboob/tools/browser2/browser.py @@ -333,6 +333,9 @@ class BaseBrowser(object): if cookies is None: cookies = self.cookies.for_request(url) kwargs['cookies'] = cookies + # erase all cookies, python-requests does not handle them securely + # and tries to merge them with provided cookies! + self.session.cookies.clear() # call python-requests response = self.session.request(method, url, **kwargs) @@ -343,7 +346,8 @@ class BaseBrowser(object): if allow_redirects: response = self.follow_redirects(response, orig_args) - # erase all cookies, python-requests does not handle them securely + # erase all cookies again + # to prevent leakage when using session.request() directly self.session.cookies.clear() return response diff --git a/weboob/tools/browser2/test.py b/weboob/tools/browser2/test.py index ce8c2997..6d7d2177 100644 --- a/weboob/tools/browser2/test.py +++ b/weboob/tools/browser2/test.py @@ -20,6 +20,7 @@ from __future__ import absolute_import from datetime import datetime +from random import choice from requests import HTTPError from nose.plugins.skip import SkipTest @@ -35,6 +36,12 @@ HTTPBIN = 'http://httpbin.org/' # https://github.com/kennethreitz/httpbin POSTBIN = 'http://www.postbin.org/' # https://github.com/progrium/postbin REQUESTBIN = 'http://requestb.in/' # https://github.com/progrium/requestbin +# if you change HTTPBIN, you should also change these URLs for some tests: +# redirect to http://httpbin.org/get +REDIRECTS1 = ('http://tinyurl.com/ouiboube-b2', 'http://bit.ly/st4Hcv') +# redirect to http://httpbin.org/cookies +REDIRECTS2 = ('http://tinyurl.com/7zp3jnr', 'http://bit.ly/HZCCX7') + def test_base(): b = BaseBrowser() @@ -99,9 +106,21 @@ def test_redirects(): assert r.status_code == 302 +def test_redirect2(): + """ + More redirect tests + """ + rurl = choice(REDIRECTS1) + b = BaseBrowser() + r = b.location(rurl) + assert r.url == HTTPBIN + 'get' + assert json.loads(r.text)['headers'].get('Referer') == rurl + # TODO referrer privacy settings + + def test_brokenpost(): """ - Tests _fix_redirect() + Test empty POST and redirect after POST """ raise SkipTest('PostBin is disabled') try: @@ -234,6 +253,9 @@ def test_referrer(): def test_cookieparse(): + """ + Test cookie parsing and processing + """ cj = CookieJar() def bc(data): @@ -284,6 +306,9 @@ def test_cookieparse(): def test_cookiejar(): + """ + Test adding, removing, finding cookies to and from the jar + """ def bc(data): """ build one cookie @@ -370,10 +395,10 @@ def test_cookiejar(): def test_buildcookie(): + """ + Test easy cookie building + """ cj = CookieJar() - """ - Test cookie building - """ c = cj.build('kk', 'vv', 'http://example.com/') assert c.domain == 'example.com' assert not c.secure @@ -435,3 +460,37 @@ def test_cookienav(): r = b.location(HTTPBIN + 'cookies') assert len(json.loads(r.text)['cookies']) == 2 assert 'bla' not in json.loads(r.text)['cookies'] + + +def test_cookieredirect(): + """ + Test cookie redirection security + """ + rurl = choice(REDIRECTS2) + + b = BaseBrowser() + r = b.location(HTTPBIN + 'cookies') + assert len(json.loads(r.text)['cookies']) == 0 + + # add a cookie to the redirection service domain (not the target!) + cookie = b.cookies.build('k', 'v1', rurl) + b.cookies.set(cookie) + r = b.location(rurl) + assert r.url == HTTPBIN + 'cookies' + # the cookie was not forwarded; it's for another domain + # this is important for security reasons, + # and because python-requests tries to do it by default! + print json.loads(r.text)['cookies'] + assert len(json.loads(r.text)['cookies']) == 0 + + # add a cookie for the target + cookie = b.cookies.build('k', 'v2', HTTPBIN) + b.cookies.set(cookie) + r = b.location(rurl) + assert r.url == HTTPBIN + 'cookies' + assert len(json.loads(r.text)['cookies']) == 1 + assert json.loads(r.text)['cookies']['k'] == 'v2' + + # check all cookies sent in the request chain + assert r.cookies == {'k': 'v2'} + assert r.history[0].cookies['k'] == 'v1' # some services add other cookies