From 46db7d2b7422cd90c03bb6534ba408b7a3b5a072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Lh=C3=A9rondelle?= Date: Fri, 15 Sep 2017 10:01:48 +0200 Subject: [PATCH 1/4] Add django rendering from email template --- phishing/helpers.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/phishing/helpers.py b/phishing/helpers.py index e3a4720..e4437c0 100644 --- a/phishing/helpers.py +++ b/phishing/helpers.py @@ -8,6 +8,8 @@ from django.core.mail import EmailMultiAlternatives from django.core.mail.backends.smtp import EmailBackend from django.core.urlresolvers import reverse +from django.template import Context +from django.template import Template from django.template.loader import render_to_string from django.utils.translation import ugettext_lazy as _ from pyshorteners import Shortener @@ -220,16 +222,12 @@ def replace_template_vars(template, campaign=None, target=None, :param email_template: `.models.EmailTemplate` :return: content with value """ + vars_context={} for var in get_template_vars(campaign, target, email_template): - names = ( - '{{%s}}' % var['name'], - '{{ %s }}' % var['name'] - ) - value = var['value'] or '' - for name in names: - template = template.replace(name, value) - - return template + vars_context[var['name']]=var['value'] + + context = Context(vars_context) + return Template(template).render(context) def start_campaign(campaign): From 9066259677016dd52de958a69d79eb3cc0fdd701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Lh=C3=A9rondelle?= Date: Fri, 15 Sep 2017 10:28:24 +0200 Subject: [PATCH 2/4] Improve performance --- phishing/helpers.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/phishing/helpers.py b/phishing/helpers.py index e4437c0..eafaad7 100644 --- a/phishing/helpers.py +++ b/phishing/helpers.py @@ -222,11 +222,8 @@ def replace_template_vars(template, campaign=None, target=None, :param email_template: `.models.EmailTemplate` :return: content with value """ - vars_context={} - for var in get_template_vars(campaign, target, email_template): - vars_context[var['name']]=var['value'] - - context = Context(vars_context) + vars = get_template_vars(campaign, target, email_template) + context = Context({v['name']: v['value'] for v in vars }) return Template(template).render(context) From 018cea70b86c3b7d74fb3592e5b7a1edf44cdc7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Lh=C3=A9rondelle?= Date: Fri, 15 Sep 2017 11:25:30 +0200 Subject: [PATCH 3/4] Add tests (security test don't work with load injection) --- phishing/tests/signal.py | 4 ++-- phishing/tests/template.py | 21 ++++++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/phishing/tests/signal.py b/phishing/tests/signal.py index 0f15b7b..aca76b5 100644 --- a/phishing/tests/signal.py +++ b/phishing/tests/signal.py @@ -100,9 +100,9 @@ def handler(vars_data, **kwarg): var_data = self.get('email') self.assertIsNone(var_data) - # test replace + # test render content = replace_template_vars('{{ email }}') - self.assertEqual(content, '{{ email }}') + self.assertEqual(content, '') # clean self.assertTrue(make_template_vars.disconnect(handler)) diff --git a/phishing/tests/template.py b/phishing/tests/template.py index 6a284be..e8c0518 100644 --- a/phishing/tests/template.py +++ b/phishing/tests/template.py @@ -4,7 +4,8 @@ from django.test import TestCase from django.urls import reverse -from phishing.helpers import minimize_url, get_template_vars +from phishing.helpers import minimize_url, get_template_vars, \ + replace_template_vars from phishing.models import Target, TargetGroup, EmailTemplate from phishing.tests.constant import FIXTURE_PATH @@ -240,3 +241,21 @@ def test_delete_email_template_permissions(self): self.assertEqual(resp.status_code, 200) # TODO: Test user that is not admin + + def test_security_template(self): + # test render + content = replace_template_vars('{{ request }}') + print(content) + self.assertEqual(content, '') + + # test render + content = replace_template_vars("{% include 'phishing/email/tracker_image.html' %}") + self.assertEqual(content, '') + + # test load + content = replace_template_vars('{% load log %}{% get_admin_log 10 as admin_log %}{{ admin_log }}') + self.assertEqual(content, '') + + # list load avalible + content = replace_template_vars('{% load fghdfhfghd %}') + self.assertEqual(content, '') From e9ceb2026f3fa6821032a73a0171d8a1842beb98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Lh=C3=A9rondelle?= Date: Mon, 23 Oct 2017 15:41:45 +0200 Subject: [PATCH 4/4] Add sandbowing to avoid security issues --- phishing/helpers.py | 13 +++++++++---- phishing/tests/signal.py | 8 ++++---- phishing/tests/template.py | 26 +++++++++++++++++--------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/phishing/helpers.py b/phishing/helpers.py index eafaad7..88a0fd0 100644 --- a/phishing/helpers.py +++ b/phishing/helpers.py @@ -12,6 +12,7 @@ from django.template import Template from django.template.loader import render_to_string from django.utils.translation import ugettext_lazy as _ +from jinja2.sandbox import SandboxedEnvironment from pyshorteners import Shortener from mercure.settings import HOSTNAME @@ -212,7 +213,7 @@ def minimize_url(url): return Shortener('Tinyurl', timeout=10.0).short(url) if url else '' -def replace_template_vars(template, campaign=None, target=None, +def render_jinja2(template, campaign=None, target=None, email_template=None): """Replace vars in template @@ -223,8 +224,12 @@ def replace_template_vars(template, campaign=None, target=None, :return: content with value """ vars = get_template_vars(campaign, target, email_template) - context = Context({v['name']: v['value'] for v in vars }) - return Template(template).render(context) + env = SandboxedEnvironment() + context = {} + for v in vars: + context[v['name']] = v['value'] + tpl = env.from_string(template) + return tpl.render(context) def start_campaign(campaign): @@ -260,7 +265,7 @@ def add_tracker(key, value, infos=None): # replace template vars helper function def replace_vars(content): - return replace_template_vars(content, campaign, target, + return render_jinja2(content, campaign, target, email_template) # send email diff --git a/phishing/tests/signal.py b/phishing/tests/signal.py index aca76b5..150a2d1 100644 --- a/phishing/tests/signal.py +++ b/phishing/tests/signal.py @@ -10,7 +10,7 @@ from django.urls import reverse from shutil import copyfile -from phishing.helpers import get_template_vars, replace_template_vars +from phishing.helpers import get_template_vars, render_jinja2 from phishing.models import Attachment, Campaign, EmailTemplate, LandingPage, \ Target, TargetGroup, Tracker, TrackerInfos from phishing.signals import make_campaign_report, make_menu, \ @@ -60,7 +60,7 @@ def handler(vars_data, **kwarg): self.assertEqual(var_data['description'], 'Is a test!') # test replace - content = replace_template_vars('{{ test_var }}') + content = render_jinja2('{{ test_var }}') self.assertEqual(content, 'Hello!') # clean @@ -81,7 +81,7 @@ def handler(vars_data, **kwarg): self.assertEqual(var_data['value'], 'Hello Word!') # test replace - content = replace_template_vars('{{ email }}') + content = render_jinja2('{{ email }}') self.assertEqual(content, 'Hello Word!') # clean @@ -101,7 +101,7 @@ def handler(vars_data, **kwarg): self.assertIsNone(var_data) # test render - content = replace_template_vars('{{ email }}') + content = render_jinja2('{{ email }}') self.assertEqual(content, '') # clean diff --git a/phishing/tests/template.py b/phishing/tests/template.py index e8c0518..e811676 100644 --- a/phishing/tests/template.py +++ b/phishing/tests/template.py @@ -5,7 +5,7 @@ from django.urls import reverse from phishing.helpers import minimize_url, get_template_vars, \ - replace_template_vars + render_jinja2 from phishing.models import Target, TargetGroup, EmailTemplate from phishing.tests.constant import FIXTURE_PATH @@ -244,18 +244,26 @@ def test_delete_email_template_permissions(self): def test_security_template(self): # test render - content = replace_template_vars('{{ request }}') - print(content) + content = render_jinja2('{{ request }}') self.assertEqual(content, '') # test render - content = replace_template_vars("{% include 'phishing/email/tracker_image.html' %}") - self.assertEqual(content, '') + with self.assertRaises(Exception) as context: + render_jinja2( + "{% include 'phishing/email/tracker_image.html' %}") + self.assertTrue('no loader for this environment specified' + in str(context.exception)) # test load - content = replace_template_vars('{% load log %}{% get_admin_log 10 as admin_log %}{{ admin_log }}') - self.assertEqual(content, '') + with self.assertRaises(Exception) as context: + render_jinja2( + '{% load log %}{% get_admin_log 10 as admin_log %}{{ admin_log }}') + self.assertTrue('Encountered unknown tag \'load\'.' + in str(context.exception)) # list load avalible - content = replace_template_vars('{% load fghdfhfghd %}') - self.assertEqual(content, '') + with self.assertRaises(Exception) as context: + render_jinja2('{% load fghdfhfghd %}') + self.assertTrue('Encountered unknown tag \'load\'.' + in str(context.exception)) +