From 187a1db85e8a277ea8e9834de21365606aa07e11 Mon Sep 17 00:00:00 2001 From: Goury Date: Wed, 26 Jul 2023 14:15:38 +0300 Subject: [PATCH 1/3] Allow relative URLs, better security --- precise_bbcode/bbcode/defaults/tag.py | 29 +++++++++++++++------------ 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/precise_bbcode/bbcode/defaults/tag.py b/precise_bbcode/bbcode/defaults/tag.py index ca202c7..78ccc5e 100644 --- a/precise_bbcode/bbcode/defaults/tag.py +++ b/precise_bbcode/bbcode/defaults/tag.py @@ -111,19 +111,22 @@ def render(self, value, option=None, parent=None): href = href[1:-1] href = replace(href, bbcode_settings.BBCODE_ESCAPE_HTML) if '://' not in href and self._domain_re.match(href): - href = 'http://' + href - v = URLValidator() - - # Validates and renders the considered URL. - try: - v(href) - except ValidationError: - rendered = '[url={}]{}[/url]'.format(href, value) if option else \ - '[url]{}[/url]'.format(value) - else: - content = value if option else href - rendered = '{}'.format(href, content or href) - + href = 'https://' + href + + if href[:2] == '//': + # Protocolless absolute URLs are unsafe. + href = '' + + if '://' in href: + # Validates the considered URL only if it is not relative. + v = URLValidator() + try: + v(href) + except ValidationError: + href = '' + + content = value if option else href + rendered = '{}'.format(href, content or href) return rendered From 0b68261269843d915f3e9955fae7109375132bec Mon Sep 17 00:00:00 2001 From: Goury Date: Wed, 26 Jul 2023 14:23:44 +0300 Subject: [PATCH 2/3] Allow relative URLs everywhere --- precise_bbcode/bbcode/defaults/placeholder.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/precise_bbcode/bbcode/defaults/placeholder.py b/precise_bbcode/bbcode/defaults/placeholder.py index 7bc21ad..d686236 100644 --- a/precise_bbcode/bbcode/defaults/placeholder.py +++ b/precise_bbcode/bbcode/defaults/placeholder.py @@ -30,11 +30,14 @@ class UrlBBCodePlaceholder(BBCodePlaceholder): name = 'url' def validate(self, content, extra_context=None): - v = URLValidator() - try: - v(content) - except ValidationError: + if content[:2] == '//': return False + if '://' in content: + v = URLValidator() + try: + v(content) + except ValidationError: + return False return True From 3847acbd17cbcfd584ab5a438181764678ff986d Mon Sep 17 00:00:00 2001 From: Goury Date: Sun, 30 Jul 2023 12:28:28 +0300 Subject: [PATCH 3/3] more tests, fix old tests, restore url replacing behaviour, better xss filtering --- precise_bbcode/bbcode/defaults/placeholder.py | 5 ++- precise_bbcode/bbcode/defaults/tag.py | 19 ++++++++--- precise_bbcode/bbcode/parser.py | 17 ++++++++-- precise_bbcode/conf/settings.py | 2 ++ tests/unit/test_parser.py | 32 ++++++++++++------- 5 files changed, 57 insertions(+), 18 deletions(-) diff --git a/precise_bbcode/bbcode/defaults/placeholder.py b/precise_bbcode/bbcode/defaults/placeholder.py index d686236..6004f3c 100644 --- a/precise_bbcode/bbcode/defaults/placeholder.py +++ b/precise_bbcode/bbcode/defaults/placeholder.py @@ -3,9 +3,9 @@ from django.core.exceptions import ValidationError from django.core.validators import URLValidator +from precise_bbcode.conf import settings as bbcode_settings from precise_bbcode.bbcode.placeholder import BBCodePlaceholder - __all__ = [ 'UrlBBCodePlaceholder', 'EmailBBCodePlaceholder', @@ -30,6 +30,9 @@ class UrlBBCodePlaceholder(BBCodePlaceholder): name = 'url' def validate(self, content, extra_context=None): + for xss in bbcode_settings.URL_XSS_FILTER: + if xss in content: + return False if content[:2] == '//': return False if '://' in content: diff --git a/precise_bbcode/bbcode/defaults/tag.py b/precise_bbcode/bbcode/defaults/tag.py index 78ccc5e..caedc27 100644 --- a/precise_bbcode/bbcode/defaults/tag.py +++ b/precise_bbcode/bbcode/defaults/tag.py @@ -104,18 +104,30 @@ class Options: replace_links = False def render(self, value, option=None, parent=None): + def bad_value_render(href): + if option: + return '[url={}]{}[/url]'.format(href, value) + else: + return '[url]{}[/url]'.format(value) + href = option if option else value if href[0] == href[-1] and href[0] in ('"', '\'') and len(href) > 2: # URLs can be encapsulated in quotes (either single or double) that aren't part of the # URL. If that's the case, strip them out. href = href[1:-1] + if not href: + return bad_value_render(href) href = replace(href, bbcode_settings.BBCODE_ESCAPE_HTML) + for xss in bbcode_settings.URL_XSS_FILTER: + if xss in href: + return bad_value_render(href) + if '://' not in href and self._domain_re.match(href): href = 'https://' + href if href[:2] == '//': # Protocolless absolute URLs are unsafe. - href = '' + return bad_value_render(href) if '://' in href: # Validates the considered URL only if it is not relative. @@ -123,11 +135,10 @@ def render(self, value, option=None, parent=None): try: v(href) except ValidationError: - href = '' + return bad_value_render(href) content = value if option else href - rendered = '{}'.format(href, content or href) - return rendered + return '{}'.format(href, content or href) class ImgBBCodeTag(BBCodeTag): diff --git a/precise_bbcode/bbcode/parser.py b/precise_bbcode/bbcode/parser.py index 96bf6e7..57b945c 100644 --- a/precise_bbcode/bbcode/parser.py +++ b/precise_bbcode/bbcode/parser.py @@ -1,6 +1,8 @@ import re from collections import defaultdict +from django.core.validators import URLValidator + from precise_bbcode.bbcode.regexes import url_re from precise_bbcode.conf import settings as bbcode_settings from precise_bbcode.core.utils import replace @@ -402,8 +404,19 @@ def _render_textual_content(self, data, replace_specialchars, replace_links, rep if replace_links: def linkrepl(match): url = match.group(0) - href = url if '://' in url else 'http://' + url - return '{1}'.format(href, url) + for xss in bbcode_settings.URL_XSS_FILTER: + if xss in url: + return url + if url[:2] == '//': + return url + if '://' in url: + v = URLValidator() + try: + v(url) + except ValidationError: + return url + + return '{1}'.format(url, url) data = re.sub(url_re, linkrepl, data) if replace_smilies: diff --git a/precise_bbcode/conf/settings.py b/precise_bbcode/conf/settings.py index db74e24..473b863 100644 --- a/precise_bbcode/conf/settings.py +++ b/precise_bbcode/conf/settings.py @@ -30,3 +30,5 @@ # Smileys options BBCODE_ALLOW_SMILIES = getattr(settings, 'BBCODE_ALLOW_SMILIES', True) SMILIES_UPLOAD_TO = getattr(settings, 'BBCODE_SMILIES_UPLOAD_TO', 'precise_bbcode/smilies') + +URL_XSS_FILTER = '"<>^`{|}();' diff --git a/tests/unit/test_parser.py b/tests/unit/test_parser.py index c9fd2ac..0829dce 100644 --- a/tests/unit/test_parser.py +++ b/tests/unit/test_parser.py @@ -32,23 +32,25 @@ class TestParser(object): '[url]http://www.google.com[/url]', 'http://www.google.com' ), - ('[url=google.com]goto google[/url]', 'goto google'), + ('[url=google.com]goto google[/url]', 'goto google'), ('[url=http://google.com][/url]', 'http://google.com'), ('[url=\'http://google.com\'][/url]', 'http://google.com'), ('[url="http://google.com"][/url]', 'http://google.com'), - ('[URL=google.com]goto google[/URL]', 'goto google'), + ('[URL=google.com]goto google[/URL]', 'goto google'), + ('[URL=/localhost/www][/URL]', '/localhost/www'), + ('[URL=/localhost/www]goto localhost[/URL]', 'goto localhost'), ( '[url=]xss[/url]', '[url=<script>alert(1);</script>]xss[/url]' ), ( 'www.google.com foo.com/bar http://xyz.ci', - 'www.google.com ' - 'foo.com/bar http://xyz.ci' + 'www.google.com ' + 'foo.com/bar http://xyz.ci' ), - ('[url=relative/foo/bar.html]link[/url]', '[url=relative/foo/bar.html]link[/url]'), - ('[url=/absolute/foo/bar.html]link[/url]', '[url=/absolute/foo/bar.html]link[/url]'), - ('[url=./hello.html]world![/url]', '[url=./hello.html]world![/url]'), + ('[url=relative/foo/bar.html]link[/url]', 'link'), + ('[url=/absolute/foo/bar.html]link[/url]', 'link'), + ('[url=./hello.html]world![/url]', 'world!'), ( '[url=javascript:alert(String.fromCharCode(88,83,83))]http://google.com[/url]', '[url=javascript:alert(String.fromCharCode(88,83,83))]http://google.com[/url]' @@ -73,11 +75,19 @@ class TestParser(object): '[img]http://foo.com/fake.png [img] ' 'onerrorjavascript:alert(String.fromCharCode(88,83,83)) [/img] [/img]' ), + ( + '[img]/localhost/www[/img]', + '' + ), + ( + '[url=/localhost/www][img]/localhost/www[/img][/url]', + '' + ), ('[quote] \r\nhello\nworld! [/quote]', '
hello
world!
'), ('[code][b]hello world![/b][/code]', '[b]hello world![/b]'), ( '[color=green]goto [url=google.com]google website[/url][/color]', - 'goto google website' + 'goto google website' ), ('[color=#FFFFFF]white[/color]', 'white'), ( @@ -142,8 +152,8 @@ class TestParser(object): 'definition_string': '[youtube]{TEXT}[/youtube]', 'format_string': ( '' ), @@ -208,7 +218,7 @@ class TestParser(object): ), ( '[youtube]ztD3mRMdqSw[/youtube]', - '' # noqa + '' # noqa ), ( '[h1=#FFF]hello world![/h1]',