Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Prevent external redirects
  • Loading branch information
sissbruecker committed Mar 27, 2022
1 parent eca98a1 commit 3906d9e
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 9 deletions.
20 changes: 14 additions & 6 deletions bookmarks/tests/test_bookmark_action_view.py
Expand Up @@ -74,7 +74,6 @@ def test_delete_should_delete_bookmark(self):

self.assertEqual(Bookmark.objects.count(), 0)


def test_delete_can_only_delete_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark = self.setup_bookmark(user=other_user)
Expand Down Expand Up @@ -305,10 +304,19 @@ def test_should_not_redirect_to_external_url(self):
bookmark2 = self.setup_bookmark()
bookmark3 = self.setup_bookmark()

url = reverse('bookmarks:action') + '?return_url=https://example.com'
response = self.client.post(url, {
'bulk_archive': [''],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
})
def post_with(return_url, follow=None):
url = reverse('bookmarks:action') + f'?return_url={return_url}'
return self.client.post(url, {
'bulk_archive': [''],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
}, follow=follow)

response = post_with('https://example.com')
self.assertRedirects(response, reverse('bookmarks:index'))
response = post_with('//example.com')
self.assertRedirects(response, reverse('bookmarks:index'))
response = post_with('://example.com')
self.assertRedirects(response, reverse('bookmarks:index'))

response = post_with('/foo//example.com', follow=True)
self.assertEqual(response.status_code, 404)
14 changes: 12 additions & 2 deletions bookmarks/tests/test_bookmark_edit_view.py
Expand Up @@ -88,12 +88,22 @@ def test_should_redirect_to_bookmark_index_by_default(self):

def test_should_not_redirect_to_external_url(self):
bookmark = self.setup_bookmark()
form_data = self.create_form_data({'return_url': 'https://example.com'})

response = self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data)
def post_with(return_url, follow=None):
form_data = self.create_form_data()
url = reverse('bookmarks:edit', args=[bookmark.id]) + f'?return_url={return_url}'
return self.client.post(url, form_data, follow=follow)

response = post_with('https://example.com')
self.assertRedirects(response, reverse('bookmarks:index'))
response = post_with('//example.com')
self.assertRedirects(response, reverse('bookmarks:index'))
response = post_with('://example.com')
self.assertRedirects(response, reverse('bookmarks:index'))

response = post_with('/foo//example.com', follow=True)
self.assertEqual(response.status_code, 404)

def test_can_only_edit_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark = self.setup_bookmark(user=other_user)
Expand Down
3 changes: 2 additions & 1 deletion bookmarks/utils.py
@@ -1,3 +1,4 @@
import re
from datetime import datetime
from typing import Optional

Expand Down Expand Up @@ -99,6 +100,6 @@ def parse_timestamp(value: str):

def get_safe_return_url(return_url: str, fallback_url: str):
# Use fallback if URL is none or URL is not on same domain
if not return_url or not return_url.startswith('/'):
if not return_url or not re.match(r'^/[a-z]+', return_url):
return fallback_url
return return_url

0 comments on commit 3906d9e

Please sign in to comment.