[#8536] don't use jinja for site notifications; add autoescape
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index bc52a56..26dd2d9 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -790,10 +790,26 @@
@pass_context
-def subrender_jinja_filter(context, value):
- _template = context.eval_ctx.environment.from_string(value)
- result = _template.render(**context)
- return result
+def subrender_jinja_filter(context, html_tmpl: str) -> Markup:
+ # jinja templates can execute potentially dangerous things
+ # _template = context.eval_ctx.environment.from_string(html_tmpl)
+ # return _template.render(**context)
+
+ # so instead, support just a few things
+
+ limited_vars = {
+ '{{ c.project.url() }}': lambda: c.project.url(),
+ }
+ for var, fn in limited_vars.items():
+ if var not in html_tmpl:
+ continue
+ try:
+ val = fn()
+ except Exception:
+ log.exception(f'Could not replace {var} in jinja "subrender" for site notification')
+ continue
+ html_tmpl = html_tmpl.replace(var, val)
+ return Markup(html_tmpl)
def nl2br_jinja_filter(value):
diff --git a/Allura/allura/lib/widgets/search.py b/Allura/allura/lib/widgets/search.py
index cec39ad..c772992 100644
--- a/Allura/allura/lib/widgets/search.py
+++ b/Allura/allura/lib/widgets/search.py
@@ -53,6 +53,7 @@
super().__init__()
# can't use g.jinja2_env since this widget gets imported too early :(
jinja2_env = jinja2.Environment(
+ autoescape=True,
loader=jinja2.PackageLoader('allura', 'templates/widgets'))
self.content = Markup(jinja2_env.get_template('search_help.html').render(dict(
comments=comments,
diff --git a/Allura/allura/public/nf/js/allura-base.js b/Allura/allura/public/nf/js/allura-base.js
index 779f4eb..839408d 100644
--- a/Allura/allura/public/nf/js/allura-base.js
+++ b/Allura/allura/public/nf/js/allura-base.js
@@ -209,7 +209,7 @@
});
$('#site-notification .btn-close').click(function(e) {
- var $note = $(this).parent();
+ var $note = $(this).parents('section:first');
$note.hide();
var note_id = $note.attr('data-notification-id');
var cookie = $.cookie('site-notification');
diff --git a/Allura/allura/templates/jinja_master/theme_macros.html b/Allura/allura/templates/jinja_master/theme_macros.html
index e06f5d7..c9ee789 100644
--- a/Allura/allura/templates/jinja_master/theme_macros.html
+++ b/Allura/allura/templates/jinja_master/theme_macros.html
@@ -178,7 +178,7 @@
{% if note %}
<div id="site-notification">
<section class="site-message info" data-notification-id="{{note._id}}">
- {{ note.content|subrender|safe }}
+ {{ note.content|subrender }}
<a href="" class="btn btn-close">Close</a>
</section>
</div>
diff --git a/Allura/allura/templates_responsive/jinja_master/theme_macros.html b/Allura/allura/templates_responsive/jinja_master/theme_macros.html
index 5c63911..dd0e70e 100644
--- a/Allura/allura/templates_responsive/jinja_master/theme_macros.html
+++ b/Allura/allura/templates_responsive/jinja_master/theme_macros.html
@@ -195,7 +195,7 @@
{% if note %}
<div id="site-notification">
<section class="callout primary" data-notification-id="{{note._id}}">
- {{note.content|safe}}
+ {{ note.content|subrender }}
<button class="close-button btn-close" aria-label="Dismiss alert" type="button">
{# .btn-close instead of data-close, since allura-base.js handles closing it, not Foundation #}
<span aria-hidden="true">×</span>
diff --git a/Allura/allura/tests/test_helpers.py b/Allura/allura/tests/test_helpers.py
index 9a12062..bb7908c 100644
--- a/Allura/allura/tests/test_helpers.py
+++ b/Allura/allura/tests/test_helpers.py
@@ -24,6 +24,8 @@
import PIL
from mock import Mock, patch
from tg import tmpl_context as c
+from tg import config
+
from alluratest.tools import module_not_available
from webob import Request
from webob.exc import HTTPUnauthorized
@@ -346,6 +348,23 @@
Markup('foo<script>alert(1)</script><br>\nbar<br>\nbaz'))
+def test_subrender_jinja_filter():
+ # if we need a real ctx, have to do all this which probably isn't even quite right
+ # from allura.config.app_cfg import AlluraJinjaRenderer
+ # j2env = AlluraJinjaRenderer.create(config, None)['jinja'].jinja2_env
+ # from jinja2.runtime import new_context
+ # j_ctx = new_context(j2env, template_name=None, blocks={})
+ j_ctx = None
+
+ # HTML, but no jinja:
+ assert h.subrender_jinja_filter(j_ctx, '<b>hi</b>{% foo %}') == '<b>hi</b>{% foo %}'
+ # var does not error if no `c.project`
+ assert h.subrender_jinja_filter(j_ctx, '<a href="{{ c.project.url() }}"></a>') == '<a href="{{ c.project.url() }}"></a>'
+ # with `c.project` set, replacement of this var works:
+ with h.push_context('test', neighborhood='Projects'):
+ assert h.subrender_jinja_filter(j_ctx, '<a href="{{ c.project.url() }}"></a>') == '<a href="/p/test/"></a>'
+
+
def test_split_select_field_options():
assert (h.split_select_field_options('"test message" test2') ==
['test message', 'test2'])
