You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by gc...@apache.org on 2026/10/05 16:22:55 UTC
(allura) branch gc/8628 created (now fb1f4c5be)
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a change to branch gc/8628
in repository https://gitbox.apache.org/repos/asf/allura.git
at fb1f4c5be bumping EasyWidgets 0.4.3 -> 0.5.0, reverting some security commits that are now included in the package itself and adding a couple of tests
This branch includes the following new commits:
new 06cd15a48 Revert "[#8623] bound the easywidgets resource cache and cap _slim requests"
new acfecd6b8 Revert "[#8623] keep inline script/style bodies raw under easywidgets autoescaping"
new fb1f4c5be bumping EasyWidgets 0.4.3 -> 0.5.0, reverting some security commits that are now included in the package itself and adding a couple of tests
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(allura) 03/03: bumping EasyWidgets 0.4.3 -> 0.5.0, reverting some security commits that are now included in the package itself and adding a couple of tests
Posted by gc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch gc/8628
in repository https://gitbox.apache.org/repos/asf/allura.git
commit fb1f4c5bea2fd8e3f00412d2bb6179d1ad297a32
Author: Guillermo Cruz <gu...@slashdotmedia.com>
AuthorDate: Mon Oct 5 15:28:19 2026 +0000
bumping EasyWidgets 0.4.3 -> 0.5.0, reverting some security commits that are now included in the package itself and adding a couple of tests
---
.../allura/tests/test_security_poc_easywidgets.py | 465 +++++++++++++++++++++
.../test_security_a1_select_options_xss.py | 145 +++++++
requirements.txt | 2 +-
3 files changed, 611 insertions(+), 1 deletion(-)
diff --git a/Allura/allura/tests/test_security_poc_easywidgets.py b/Allura/allura/tests/test_security_poc_easywidgets.py
new file mode 100644
index 000000000..f278c284e
--- /dev/null
+++ b/Allura/allura/tests/test_security_poc_easywidgets.py
@@ -0,0 +1,465 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Regression tests for EasyWidgets security fixes.
+
+Each test is a proof-of-concept asserting the *secure* outcome: it fails on EasyWidgets 0.4.3 and
+passes once the issue is fixed. Only the tracker select-option XSS is exploitable through a form
+in stock Allura; the unbounded _slim cache is an unauthenticated DoS. The rest are library
+primitives or hardening, as each class docstring says. TestKnownOpen is xfail; an XPASS means
+someone fixed it.
+
+Unit-level: no MongoDB, Solr or WSGI app needed::
+
+ cd Allura && pytest allura/tests/test_security_poc_easywidgets.py
+"""
+
+import html
+import os
+import re
+
+import ew
+import ew.jinja2_ew as jew
+import pytest
+from bs4 import BeautifulSoup
+from ew import widget_context
+from ew.core import WidgetContext
+from ew.render import TemplateEngine
+from ew.resource import ResourceManager
+from ew.utils import Bunch, push_context
+from paste.registry import Registry
+
+from allura.lib import helpers as h
+from allura.lib.widgets.forms import _HTMLExplanation
+
+try:
+ from forgetracker.widgets.ticket_form import MilestoneField, TicketCustomField
+except ImportError: # ForgeTracker not installed alongside Allura
+ MilestoneField = TicketCustomField = None
+
+needs_forgetracker = pytest.mark.skipif(TicketCustomField is None, reason='ForgeTracker not installed')
+
+# Typed into a select field's "Options" box; shlex unescapes \" so a bare " survives
+SELECT_OPTIONS_EXPLOIT = r'"x\" onmouseover=alert(document.domain) autofocus onfocus=alert(1) y"'
+
+# Breaks out of value="..." if unescaped
+ATTR_BREAKOUT = 'x" onmouseover=alert(document.domain) autofocus onfocus=alert(1) y'
+
+# Breaks out of element text if unescaped
+TEXT_BREAKOUT = '<img src=x onerror=alert(document.domain)>'
+
+
+@pytest.fixture(autouse=True)
+def ew_widget_context():
+ """Per-request widget context and clean ResourceManager, as WidgetMiddleware sets up; restored after."""
+ saved_engines = TemplateEngine._engines
+ saved_paths = ResourceManager.paths
+ saved_cache = ResourceManager.resource_cache
+ TemplateEngine.initialize({})
+ ResourceManager.paths = []
+ ResourceManager.resource_cache = {}
+ registry = Registry()
+ registry.prepare()
+ manager = ResourceManager()
+ registry.register(widget_context, WidgetContext(scheme='http', resource_manager=manager))
+ yield manager
+ registry.cleanup()
+ TemplateEngine._engines = saved_engines
+ ResourceManager.paths = saved_paths
+ ResourceManager.resource_cache = saved_cache
+
+
+def soup(markup):
+ return BeautifulSoup(markup, 'html.parser')
+
+
+def event_handler_attrs(markup):
+ """Every on*= attribute the browser would parse out of the markup."""
+ found = set()
+ for tag in soup(markup).find_all():
+ found.update(name for name in tag.attrs if name.startswith('on'))
+ return found
+
+
+class TestTemplateTextEvaluatedAsPython:
+ """CoreEngine resolved ${...} with a bare eval(), so any text rendered through
+ the default core-ew engine (Snippet(text), Widget.expand()) ran as code.
+
+ Reachability: latent. Nothing in this tree feeds attacker-controlled text to core-ew.
+ """
+
+ def test_snippet_does_not_import_modules(self):
+ rendered = ew.Snippet('${__import__("os")}')({})
+ assert 'module' not in rendered
+ assert 'posix' not in rendered
+
+ def test_snippet_does_not_execute_calls(self, tmp_path):
+ canary = tmp_path / 'ew1_snippet.txt'
+ payload = f'${{__import__("os").popen("touch {canary}").read()}}'
+ ew.Snippet(payload)({})
+ assert not canary.exists()
+
+ def test_html_field_does_not_execute_calls(self, tmp_path):
+ """HTMLField runs its text through expand(), so the eval() primitive is reachable from a widget."""
+ canary = tmp_path / 'ew1_htmlfield.txt'
+ payload = f'${{__import__("os").popen("touch {canary}").read()}}'
+ jew.HTMLField(text=payload).display()
+ assert not canary.exists()
+
+ def test_html_escape_is_not_a_defense(self, tmp_path):
+ """A payload built from bytes([...]).decode() has nothing for html.escape() to change."""
+ canary = tmp_path / 'ew1_escaped.txt'
+ cmd = f'touch {canary}'
+ payload = '${{__import__(bytes({os_!r}).decode()).popen(bytes({cmd_!r}).decode()).read()}}'.format(
+ os_=list(b'os'), cmd_=list(cmd.encode()))
+ escaped = html.escape(payload)
+ assert escaped == payload
+ jew.HTMLField(text=escaped).display()
+ assert not canary.exists()
+
+ def test_dunder_attribute_access_is_rejected(self):
+ rendered = ew.Snippet('${obj.__class__}')(dict(obj=object()))
+ assert 'type' not in rendered
+ assert 'object' not in rendered
+
+ @pytest.mark.parametrize(('template', 'context', 'expected'), [
+ ('${value+4}', dict(value=1), '${value+4}'),
+ ('${name}', dict(name='bob'), 'bob'),
+ ("${obj['k']}", dict(obj={'k': 'v'}), "${obj['k']}"),
+ ('${a.b}', dict(a=Bunch(b='nested')), '${a.b}'),
+ ('${x if y else z}', dict(x='yes', y=True, z='no'), '${x if y else z}'),
+ ])
+ def test_template_expression_syntax_not_supported(self, template, context, expected):
+ assert ew.Snippet(template)(context) == expected
+
+
+class TestAutoescapeOff:
+ """The widget jinja2 environment had autoescape off and the bundled templates
+ interpolated values with no |e. Widget output is Markup, so Allura's own environment passes it
+ through.
+
+ Reachability: mixed. Select-option values are live (see the tracker tests). Other sinks only get
+ developer-controlled text from Allura today (hardening), but tools outside this repo can reach them.
+ """
+
+ def test_jinja2_engine_autoescapes_by_default(self):
+ assert TemplateEngine.get_engine('jinja2')._environ.autoescape is True
+
+ def test_autoescape_is_still_configurable(self):
+ TemplateEngine.initialize({'jinja2.autoescape': False})
+ assert TemplateEngine.get_engine('jinja2')._environ.autoescape is False
+
+ def test_allura_middleware_sets_autoescape_for_the_widget_env(self):
+ """Allura must set autoescape for the widget env too, so an older EasyWidgets can't render unescaped."""
+ middleware_py = os.path.join(os.path.dirname(h.__file__), '..', 'config', 'middleware.py')
+ with open(middleware_py, encoding='utf-8') as fp:
+ source = fp.read()
+ assert re.search(r"""['"]jinja2\.autoescape['"]\s*:""", source)
+
+ def test_select_field_option_value(self):
+ """select_field.html: value="{{o.html_value}}" """
+ field = jew.SingleSelectField(
+ name='x', options=[jew.Option(label='a', py_value='a', html_value=ATTR_BREAKOUT)])
+ rendered = field.display()
+ assert event_handler_attrs(rendered) == set()
+
+ def test_checkbox_set_option_value_and_labels(self):
+ """checkbox_set.html: {{label}}, {{o.label}}, value="{{o.html_value}}" """
+ field = jew.CheckboxSet(
+ name='cs', label=TEXT_BREAKOUT,
+ options=[jew.Option(label=TEXT_BREAKOUT, py_value='v', html_value=ATTR_BREAKOUT)])
+ rendered = field.display()
+ assert event_handler_attrs(rendered) == set()
+ assert soup(rendered).find('img') is None
+
+ def test_checkbox_label(self):
+ """checkbox.html: {{label}} """
+ rendered = jew.Checkbox(name='cb', label=TEXT_BREAKOUT).display()
+ assert soup(rendered).find('img') is None
+
+ def test_ew_lib_field_label(self):
+ """_ew_lib.html subfields(): <label ...>{{field.label}}</label> """
+ form = jew.SimpleForm(fields=[jew.TextField(name='f', label=TEXT_BREAKOUT)])
+ rendered = form.display()
+ assert soup(rendered).find('img') is None
+
+ def test_ew_lib_field_errors(self):
+ """_ew_lib.html subfields(): <span ...>{{ctx.errors}}</span> """
+ form = jew.SimpleForm(fields=[jew.TextField(name='f', label='ok')])
+ rendered = form.display(errors={'f': TEXT_BREAKOUT})
+ assert soup(rendered).find('img') is None
+
+ def test_ew_lib_table_column_label(self):
+ """_ew_lib.html table(): <th>{{col.label}}</th> """
+ field = jew.TableField(name='t', fields=[jew.TextField(name='f', label=TEXT_BREAKOUT)], repetitions=1)
+ rendered = field.display()
+ assert soup(rendered).find('img') is None
+
+ def test_row_field_errors(self):
+ """row_field.html: <span ...>{{ctx.errors}}</span> """
+ field = jew.RowField(name='r', fields=[jew.TextField(name='f')])
+ rendered = field.display(errors={'f': TEXT_BREAKOUT})
+ assert soup(rendered).find('img') is None
+
+ def test_repeated_field_errors(self):
+ """repeated_field.html: <span ...>{{ctx.errors}} """
+ field = jew.RepeatedField(name='rep', field=jew.TextField(name='f'), repetitions=1)
+ rendered = field.display(errors={0: TEXT_BREAKOUT})
+ assert soup(rendered).find('img') is None
+
+ def test_js_link_href(self, ew_widget_context):
+ """JSLink's snippet: src="{{widget.href}}" """
+ link = jew.JSLink('x.js" onload=alert(document.domain) a="')
+ link.manager = ew_widget_context
+ assert event_handler_attrs(link.display()) == set()
+
+ def test_css_link_href(self, ew_widget_context):
+ """CSSLink's snippet: href="{{widget.href}}" """
+ link = jew.CSSLink('x.css" onload=alert(document.domain) a="')
+ link.manager = ew_widget_context
+ assert event_handler_attrs(link.display()) == set()
+
+ def test_google_analytics_account_cannot_break_out_of_the_script(self, ew_widget_context):
+ """google_analytics.html: inside a <script>, so the account must be a JS literal (|tojson)."""
+ ga = jew.GoogleAnalytics("UA-1'); alert(document.domain); ('")
+ ga.manager = ew_widget_context
+ assert "'); alert(document.domain); ('" not in ga.display()
+
+ def test_script_and_style_bodies_are_still_emitted_as_markup(self, ew_widget_context):
+ """JSScript/CSSScript bodies are markup by definition and must stay unescaped."""
+ script = jew.JSScript('if (a && b) { c(); }')
+ script.manager = ew_widget_context
+ assert 'a && b' in script.display()
+ style = jew.CSSScript('a > b { color: red }')
+ style.manager = ew_widget_context
+ assert 'a > b' in style.display()
+
+ def test_html_explanation_still_renders_its_markup(self):
+ """forms.py:46 relied on autoescape being off, so it needs an explicit |safe."""
+ rendered = _HTMLExplanation(text='<b>explanation</b>').display()
+ assert '<b>explanation</b>' in rendered
+
+
+class TestTrackerSelectOptionsStoredXss:
+ """split_select_field_options() stripped double quotes only when shlex failed; on
+ success shlex unescapes \" and the quote reached select_field.html's unescaped value="...".
+
+ Reachability: live, the only finding here that is. An admin types the payload into Admin ->
+ Custom Fields -> Options and it runs for every user who opens the ticket form. Milestone names
+ reach the same sink.
+ """
+
+ def test_split_select_field_options_strips_quotes_on_the_shlex_success_path(self):
+ options = h.split_select_field_options(SELECT_OPTIONS_EXPLOIT)
+ assert not any('"' in opt for opt in options)
+
+ def test_split_select_field_options_strips_quotes_on_the_shlex_failure_path(self):
+ options = h.split_select_field_options('a "b')
+ assert not any('"' in opt for opt in options)
+
+ @pytest.mark.parametrize(('raw', 'expected'), [
+ ('"one two" three', ['one two', 'three']),
+ ('*default other', ['*default', 'other']),
+ ('a b c', ['a', 'b', 'c']),
+ ])
+ def test_split_select_field_options_normal_usage(self, raw, expected):
+ assert h.split_select_field_options(raw) == expected
+
+ @needs_forgetracker
+ def test_ticket_select_custom_field_renders_no_attribute_breakout(self):
+ field = Bunch(name='_test', label='Test', type='select', options=SELECT_OPTIONS_EXPLOIT)
+ rendered = TicketCustomField.SELECTOR['select'](field).display()
+ assert event_handler_attrs(rendered) == set()
+
+ @needs_forgetracker
+ def test_milestone_field_renders_no_attribute_breakout(self):
+ """MilestoneField's own unescaped value= sink, fed by milestone names."""
+ option = jew.Option(label='v1', py_value='v1', html_value=ATTR_BREAKOUT, complete=False)
+ rendered = MilestoneField(label='M', name='m', options=[option]).display()
+ assert event_handler_attrs(rendered) == set()
+
+
+class TestPathContainment:
+ """get_filename() accepted any path that merely startswith() the registered
+ directory, so a sibling like static-private/ was reachable.
+
+ Reachability: not exploitable in stock Allura; customised deployments or third-party themes can.
+ """
+
+ @pytest.fixture
+ def tree(self, tmp_path):
+ public = tmp_path / 'static'
+ public.mkdir()
+ (public / 'app.js').write_text('ok')
+ private = tmp_path / 'static-private'
+ private.mkdir()
+ (private / 'secret.key').write_text('SECRET')
+ backup = tmp_path / 'static.bak'
+ backup.mkdir()
+ (backup / 'id_rsa').write_text('PRIVATE KEY')
+ ResourceManager.register_directory('static', str(public))
+ return tmp_path
+
+ @pytest.mark.parametrize('res_path', [
+ 'static/../../etc/passwd',
+ 'static/../static-private/secret.key',
+ 'static/../static.bak/id_rsa',
+ ])
+ def test_paths_outside_the_registered_directory_are_refused(self, tree, res_path):
+ assert ResourceManager().get_filename(res_path) is None
+
+ def test_registered_directory_is_still_served(self, tree):
+ assert ResourceManager().get_filename('static/app.js') == str(tree / 'static' / 'app.js')
+
+
+class TestUnboundedResourceCache:
+ """ResourceManager.resource_cache is shared, keyed on the raw href, and has no
+ bound or eviction. WidgetMiddleware serves /_ew_resources/ with no session required.
+
+ Reachability: live and unauthenticated, but a memory-exhaustion DoS, not an injection.
+ """
+
+ FILES_PER_REQUEST = 100
+ BIG_FILE_BYTES = 64 * 1024
+ DEFAULT_CACHE_BUDGET = 64 * 1024 * 1024
+
+ @pytest.fixture
+ def static_dir(self, tmp_path):
+ public = tmp_path / 'static'
+ public.mkdir()
+ (public / 'f0.js').write_text('x' * 1024)
+ for i in range(self.FILES_PER_REQUEST):
+ (public / f'big{i}.js').write_text('x' * self.BIG_FILE_BYTES)
+ ResourceManager.register_directory('static', str(public))
+ return public
+
+ def test_one_slim_request_cannot_concatenate_files_without_bound(self, static_dir):
+ """One small request naming the same file repeatedly (~125x amplification). With
+ max_slim_hrefs the response stops growing at the cap."""
+ big = ResourceManager(use_cache=True).serve_slim('js', ';'.join(['static/f0.js'] * 1000))
+ huge = ResourceManager(use_cache=True).serve_slim('js', ';'.join(['static/f0.js'] * 10000))
+ assert len(huge) == len(big)
+
+ def test_cache_does_not_grow_with_every_distinct_request(self, static_dir):
+ """Serve more unique content than the budget; a bounded cache must retain less than it served.
+ Each request rotates the file list, so every href is a distinct key."""
+ budget = getattr(ResourceManager, 'max_cache_bytes', self.DEFAULT_CACHE_BUDGET)
+ per_request = min(self.FILES_PER_REQUEST,
+ getattr(ResourceManager, 'max_slim_hrefs', self.FILES_PER_REQUEST))
+ names = [f'static/big{i}.js' for i in range(per_request)]
+ requests = int(budget * 1.5 / (per_request * self.BIG_FILE_BYTES)) + 1
+ assert requests <= per_request
+ served = 0
+ for i in range(requests):
+ href = ';'.join(names[i:] + names[:i])
+ served += len(ResourceManager(use_cache=True).serve_slim('js', href))
+ cached = sum(len(v) for v in ResourceManager.resource_cache.values())
+ assert served > budget
+ assert cached < served
+
+
+class TestLatentRceViaHtmlField:
+ """forms.py:606 builds ew.HTMLField(text=html.escape(cat.shortname)). Not
+ exploitable, but not because of html.escape() (see the eval() tests); h.slugify() in
+ TroveCategoryController._create strips $ { } ( ) before storage. Both halves are asserted.
+
+ Only the inner sink is exercised; the whole form needs a full app context.
+
+ Reachability: latent. _create is the only writer of shortname and always slugifies.
+ """
+
+ def test_the_html_field_sink_does_not_execute_the_shortname(self, tmp_path):
+ canary = tmp_path / 'a2.txt'
+ cmd = f'touch {canary}'
+ shortname = '${{__import__(bytes({os_!r}).decode()).popen(bytes({cmd_!r}).decode()).read()}}'.format(
+ os_=list(b'os'), cmd_=list(cmd.encode()))
+ jew.HTMLField(text=html.escape(shortname), attrs={'disabled': True, 'value': shortname}).display()
+ assert not canary.exists()
+
+ def test_slugify_strips_expression_syntax(self):
+ """The accidental defense; pinned so a slug-rule refactor is visible."""
+ slug, _ = h.slugify("${__import__('os').popen('id')}")
+ for ch in '${}()\'':
+ assert ch not in slug
+
+
+class TestPushContextNotExceptionSafe:
+ """push_context() had no try/finally, so a render error left widget_context pointing
+ at the wrong widget while the error page rendered.
+
+ Reachability: not attacker-triggered; a robustness fix.
+ """
+
+ def test_attributes_are_restored_when_the_body_raises(self):
+ class Target:
+ existing = 'original'
+
+ obj = Target()
+ with pytest.raises(RuntimeError), push_context(obj, existing='temporary', added='new'):
+ raise RuntimeError('render failed')
+ assert obj.existing == 'original'
+ assert not hasattr(obj, 'added')
+
+ def test_widget_context_is_restored_when_a_widget_raises(self):
+ class Exploding(ew.Widget):
+ def template(self, context):
+ raise RuntimeError('render failed')
+
+ before = widget_context.widget
+ with pytest.raises(RuntimeError):
+ Exploding().display()
+ assert widget_context.widget is before
+
+
+class TestInternalErrorTextRendered:
+ """'[Exception in <expr>: <message>]' was rendered into the response.
+
+ Reachability: information disclosure to anyone who can make a template expression fail.
+ """
+
+ def test_exception_detail_is_not_rendered_into_the_page(self):
+ class Boom:
+ def __getattr__(self, name):
+ raise RuntimeError('SECRET_INTERNAL_DETAIL')
+
+ rendered = ew.Snippet('${obj.attr}')(dict(obj=Boom()))
+ assert 'SECRET_INTERNAL_DETAIL' not in rendered
+
+
+class TestKnownOpen:
+ """Issues deliberately left open; xfail, so XPASS means one got fixed."""
+
+ @pytest.mark.xfail(reason='per-render state on shared Option instances is a design change')
+ def test_option_selection_does_not_persist_across_renders(self):
+ """SelectField writes selected/html_value onto Option instances that are usually shared, so
+ selection state can bleed between concurrent requests."""
+ option = jew.Option(label='a', py_value='a', html_value='a')
+ field = jew.SingleSelectField(name='x', options=[option])
+ field.display(value='a')
+ field.display(value=None)
+ assert option.selected is False
+
+ @pytest.mark.xfail(reason="LinkField javascript: guard would break legitimate javascript:void(0)")
+ def test_link_field_rejects_javascript_scheme(self):
+ rendered = jew.LinkField().display(value='javascript:alert(document.domain)')
+ assert 'javascript:' not in rendered
+
+ @pytest.mark.xfail(reason='_attr() validates values but not attribute names; every attrs dict in Allura '
+ 'is developer-controlled today')
+ def test_attr_names_cannot_inject_an_attribute(self):
+ rendered = jew.InputField(name='x', attrs={'y onmouseover=alert(document.domain)': 'z'}).display()
+ assert event_handler_attrs(rendered) == set()
diff --git a/ForgeTracker/forgetracker/tests/functional/test_security_a1_select_options_xss.py b/ForgeTracker/forgetracker/tests/functional/test_security_a1_select_options_xss.py
new file mode 100644
index 000000000..66433b869
--- /dev/null
+++ b/ForgeTracker/forgetracker/tests/functional/test_security_a1_select_options_xss.py
@@ -0,0 +1,145 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""End-to-end test for stored XSS via select custom-field options.
+
+A tracker admin POSTs the payload to the custom-fields admin endpoint and a different user's ticket
+form renders it. Library-level cases are unit-tested in
+``Allura/allura/tests/test_security_poc_easywidgets.py``.
+
+Assertions are structural: the page may still contain the payload text inside an attribute value,
+but must not contain an ``on*`` attribute the browser would parse out.
+
+Needs a full Allura runtime (MongoDB, Solr)::
+
+ cd ForgeTracker && pytest forgetracker/tests/functional/test_security_a1_select_options_xss.py
+"""
+
+from formencode.variabledecode import variable_encode
+
+from forgetracker import model as tm
+from forgetracker.tests.functional.test_root import TrackerTestController
+
+# Typed into a select field's "Options" box; shlex unescapes \" so a bare " reaches value="..."
+OPTIONS_PAYLOAD = r'"x\" onmouseover=alert(document.domain) autofocus onfocus=alert(1) y"'
+
+# Same breakout, closing the element instead of the attribute
+ELEMENT_PAYLOAD = r'"x\"><script>alert(document.domain)</script>"'
+
+# Milestone names only get .replace("/", "-"), so a quote reaches the same sink
+MILESTONE_PAYLOAD = r'1.0" onmouseover=alert(document.domain) x'
+
+FIELD_NAME = '_testselect'
+MILESTONE_FIELD_NAME = '_releases'
+
+
+def event_handler_attrs(tag):
+ """Every on*= attribute a browser would parse out of this subtree."""
+ found = set()
+ for el in [tag, *tag.find_all()]:
+ found.update(name for name in el.attrs if name.startswith('on'))
+ return found
+
+
+class TestSelectOptionsStoredXss(TrackerTestController):
+
+ def _post_select_field(self, options):
+ params = dict(
+ custom_fields=[
+ dict(name=FIELD_NAME, label='Test', type='select', options=options),
+ ],
+ open_status_names='aa bb',
+ closed_status_names='cc',
+ )
+ self.app.post('/admin/bugs/set_custom_fields', params=variable_encode(params))
+
+ def _post_milestone_field(self, milestone_name):
+ params = dict(
+ custom_fields=[
+ dict(label='releases', show_in_search='on', type='milestone',
+ milestones=[dict(name=milestone_name)]),
+ ],
+ open_status_names='aa bb',
+ closed_status_names='cc',
+ )
+ self.app.post('/admin/bugs/set_custom_fields', params=variable_encode(params))
+
+ def _select_for(self, resp, field_name):
+ """The custom field's <select>, matched by name suffix (rendered names carry a form prefix)."""
+ selects = [s for s in resp.html.find_all('select')
+ if (s.get('name') or '').endswith(field_name)]
+ assert len(selects) == 1
+ return selects[0]
+
+ def _stored_options(self, field_name):
+ for globals_ in tm.Globals.query.find().all():
+ for custom_field in globals_.custom_fields or []:
+ if custom_field['name'] == field_name:
+ return custom_field['options']
+ raise AssertionError(f'no stored custom field named {field_name}')
+
+ def test_option_value_cannot_break_out_of_the_value_attribute(self):
+ """Pre-fix, the option rendered onmouseover/autofocus/onfocus attributes."""
+ self._post_select_field(OPTIONS_PAYLOAD)
+ select = self._select_for(self.app.get('/bugs/new/'), FIELD_NAME)
+ assert event_handler_attrs(select) == set()
+ assert 'autofocus' not in select.find('option').attrs
+
+ def test_payload_reaches_every_user_who_opens_the_form(self):
+ """The admin stores it; any logged-in user (*authenticated can 'create') renders it."""
+ self._post_select_field(OPTIONS_PAYLOAD)
+ victim = self.app.get('/bugs/new/', extra_environ=dict(username='test-user'))
+ select = self._select_for(victim, FIELD_NAME)
+ assert event_handler_attrs(select) == set()
+
+ def test_payload_is_stored_verbatim_so_the_defence_must_be_at_render_time(self):
+ """Input isn't sanitised, so the fix is at render time and covers already-stored payloads."""
+ self._post_select_field(OPTIONS_PAYLOAD)
+ assert self._stored_options(FIELD_NAME) == OPTIONS_PAYLOAD
+ assert '"' in self._stored_options(FIELD_NAME)
+ select = self._select_for(self.app.get('/bugs/new/'), FIELD_NAME)
+ assert event_handler_attrs(select) == set()
+
+ def test_option_value_cannot_close_the_element_and_add_markup(self):
+ """The payload must not add elements inside the select."""
+ self._post_select_field(ELEMENT_PAYLOAD)
+ select = self._select_for(self.app.get('/bugs/new/'), FIELD_NAME)
+ assert select.find('script') is None
+ assert select.find_all('option')
+
+ def test_field_label_is_not_a_second_hole(self):
+ """The label is escaped by ticket_custom_fields.html; moving it into the widget would reopen this."""
+ params = dict(
+ custom_fields=[
+ dict(name=FIELD_NAME, label=r'<img src=x onerror=alert(document.domain)>',
+ type='select', options='one two'),
+ ],
+ open_status_names='aa bb',
+ closed_status_names='cc',
+ )
+ self.app.post('/admin/bugs/set_custom_fields', params=variable_encode(params))
+ resp = self.app.get('/bugs/new/')
+ label = resp.html.find('label', attrs={'for': lambda v: bool(v) and v.endswith(FIELD_NAME)})
+ assert label is not None
+ assert label.find('img') is None
+
+ def test_milestone_name_cannot_break_out_of_the_value_attribute(self):
+ """MilestoneField has its own value= sink. Names are escaped, not stripped, to round-trip."""
+ self._post_milestone_field(MILESTONE_PAYLOAD)
+ select = self._select_for(self.app.get('/bugs/new/'), MILESTONE_FIELD_NAME)
+ assert event_handler_attrs(select) == set()
+ assert '"' in select.find('option')['value']
diff --git a/requirements.txt b/requirements.txt
index cf2b6fd4e..f0dd8c26f 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -74,7 +74,7 @@ docutils==0.23
# -r requirements.in
# defopt
# pypeline
-easywidgets==0.4.3
+easywidgets==0.5.0
# via -r requirements.in
emoji==2.15.0
# via -r requirements.in
(allura) 02/03: Revert "[#8623] keep inline script/style bodies raw under easywidgets autoescaping"
Posted by gc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch gc/8628
in repository https://gitbox.apache.org/repos/asf/allura.git
commit acfecd6b8fe4695bc8521ee64307876fc16940e7
Author: Guillermo Cruz <gu...@slashdotmedia.com>
AuthorDate: Fri Oct 2 19:04:45 2026 +0000
Revert "[#8623] keep inline script/style bodies raw under easywidgets autoescaping"
This reverts commit c30ee2a8c03c5ddd701db2a2d609fcd3a1323b2a.
---
Allura/allura/config/middleware.py | 17 ------------
Allura/allura/tests/test_middlewares.py | 48 ---------------------------------
2 files changed, 65 deletions(-)
diff --git a/Allura/allura/config/middleware.py b/Allura/allura/config/middleware.py
index 9914e8856..008a5338f 100644
--- a/Allura/allura/config/middleware.py
+++ b/Allura/allura/config/middleware.py
@@ -32,7 +32,6 @@
import activitystream
import ew
-import ew.jinja2_ew
import formencode
from ming.odm.middleware import MingMiddleware
from beaker_session_jwt import JWTCookieSession
@@ -69,21 +68,6 @@
__all__ = ['make_app']
-def _mark_ew_script_bodies_safe():
- """Keep inline <script>/<style> bodies out of easywidgets' autoescaping.
-
- easywidgets 0.4.x renders JSScript/CSSScript through a bare ``{{widget.text}}``. That was
- harmless while its jinja env had autoescaping off, but _make_core_app turns it on, which would
- escape every inline script and style body in the site (``&&`` -> ``&&`` and so on).
- Script bodies are developer-supplied markup by definition, so mark them safe. 0.5 does this
- itself; drop this once requirements pin easywidgets >= 0.5.
- """
- ew.jinja2_ew.JSScript.WidgetClass.template = ew.jinja2_ew.Snippet(
- '<script type="text/javascript">{{widget.text|safe}}</script>', 'jinja2')
- ew.jinja2_ew.CSSScript.WidgetClass.template = ew.jinja2_ew.Snippet(
- '<style>{{widget.text|safe}}</style>', 'jinja2')
-
-
# this is webapp entry point from setup.py
def make_app(global_conf: dict, **app_conf):
override_root_module_name = app_conf.get('override_root', None)
@@ -172,7 +156,6 @@ def _make_core_app(root, global_conf: dict, **app_conf):
if config.get('override_root') != 'task':
app = SSLMiddleware(app, app_conf.get('no_redirect.pattern'),
app_conf.get('force_ssl.pattern'))
- _mark_ew_script_bodies_safe()
# Setup resource manager, widget context SOP
app = ew.WidgetMiddleware(
app,
diff --git a/Allura/allura/tests/test_middlewares.py b/Allura/allura/tests/test_middlewares.py
index b2e734f64..191e00377 100644
--- a/Allura/allura/tests/test_middlewares.py
+++ b/Allura/allura/tests/test_middlewares.py
@@ -16,15 +16,6 @@
# under the License.
from mock import MagicMock, patch
-from paste.registry import Registry
-
-import ew.jinja2_ew
-from ew import widget_context
-from ew.core import WidgetContext
-from ew.render import TemplateEngine
-from ew.resource import ResourceManager
-
-from allura.config.middleware import _mark_ew_script_bodies_safe
from allura.lib.custom_middleware import CORSMiddleware
@@ -112,42 +103,3 @@ def test_get_access_control_request_headers(self):
assert f({key: ''}) == set()
assert (f({key: 'Authorization, Accept'}) ==
{'authorization', 'accept'})
-
-
-class TestEwScriptBodiesStayRaw:
- """_make_core_app turns easywidgets' autoescaping on, which would otherwise escape every
- inline script and style body rendered through JSScript/CSSScript."""
-
- def setup_method(self):
- self.saved_engines = TemplateEngine._engines
- self.saved_js = ew.jinja2_ew.JSScript.WidgetClass.template
- self.saved_css = ew.jinja2_ew.CSSScript.WidgetClass.template
- TemplateEngine.initialize({'jinja2.autoescape': True})
- self.registry = Registry()
- self.registry.prepare()
- self.manager = ResourceManager()
- self.registry.register(
- widget_context, WidgetContext(scheme='http', resource_manager=self.manager))
- _mark_ew_script_bodies_safe()
-
- def teardown_method(self):
- self.registry.cleanup()
- TemplateEngine._engines = self.saved_engines
- ew.jinja2_ew.JSScript.WidgetClass.template = self.saved_js
- ew.jinja2_ew.CSSScript.WidgetClass.template = self.saved_css
-
- def _render(self, widget):
- widget.manager = self.manager
- return widget.display()
-
- def test_js_script_body_is_not_escaped(self):
- body = '''$(function () {$('.x').go({"a": 1}); if (a && b) { c(); }});'''
- assert body in self._render(ew.jinja2_ew.JSScript(body))
-
- def test_css_script_body_is_not_escaped(self):
- body = 'div.x table{ width: 700px; } a > b { color: red }'
- assert body in self._render(ew.jinja2_ew.CSSScript(body))
-
- def test_widget_values_are_still_escaped(self):
- rendered = ew.jinja2_ew.TextField(name='f').display(value='a" onmouseover=alert(1)')
- assert '"' in rendered
(allura) 01/03: Revert "[#8623] bound the easywidgets resource cache and cap _slim requests"
Posted by gc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch gc/8628
in repository https://gitbox.apache.org/repos/asf/allura.git
commit 06cd15a48e64580e3449ba1a9d0906d05e9310f8
Author: Guillermo Cruz <gu...@slashdotmedia.com>
AuthorDate: Fri Oct 2 19:04:45 2026 +0000
Revert "[#8623] bound the easywidgets resource cache and cap _slim requests"
This reverts commit ebb6c7c87af22db90671b2eefa206a66a55d0991.
---
Allura/allura/config/middleware.py | 54 ++++++------------------
Allura/allura/lib/custom_middleware.py | 31 --------------
Allura/allura/lib/widgets/auth_widgets.py | 12 ++++--
Allura/allura/lib/widgets/forms.py | 2 +
ForgeTracker/forgetracker/widgets/ticket_form.py | 4 +-
5 files changed, 25 insertions(+), 78 deletions(-)
diff --git a/Allura/allura/config/middleware.py b/Allura/allura/config/middleware.py
index 3e969e43d..9914e8856 100644
--- a/Allura/allura/config/middleware.py
+++ b/Allura/allura/config/middleware.py
@@ -21,7 +21,6 @@
import importlib
import mimetypes
import re
-import threading
import warnings
from tg import config
@@ -34,7 +33,6 @@
import activitystream
import ew
import ew.jinja2_ew
-import ew.resource
import formencode
from ming.odm.middleware import MingMiddleware
from beaker_session_jwt import JWTCookieSession
@@ -57,7 +55,6 @@
from allura.lib.custom_middleware import SSLMiddleware
from allura.lib.custom_middleware import StaticFilesMiddleware
from allura.lib.custom_middleware import CSRFMiddleware
-from allura.lib.custom_middleware import EwSlimRequestLimitMiddleware
from allura.lib.custom_middleware import CORSMiddleware
from allura.lib.custom_middleware import LoginRedirectMiddleware
from allura.lib.custom_middleware import RememberLoginMiddleware
@@ -72,39 +69,14 @@
__all__ = ['make_app']
-class _BoundedResourceCache(dict):
- """A byte-budgeted replacement for easywidgets' unbounded resource cache.
- drop this once requirements pin easywidgets >= 0.5.
- """
-
- def __init__(self, max_bytes):
- super().__init__()
- self.max_bytes = max_bytes
- self._bytes = 0
- self._lock = threading.Lock()
-
- def __setitem__(self, key, value):
- if len(value) > self.max_bytes:
- return
- with self._lock:
- if key in self:
- self._bytes -= len(super().__getitem__(key))
- super().__delitem__(key)
- super().__setitem__(key, value)
- self._bytes += len(value)
- while self._bytes > self.max_bytes and len(self) > 1:
- oldest = next(iter(self))
- self._bytes -= len(super().__getitem__(oldest))
- super().__delitem__(oldest)
-
-
-def _bound_ew_resource_cache(max_bytes):
- ew.resource.ResourceManager.resource_cache = _BoundedResourceCache(max_bytes)
-
-
def _mark_ew_script_bodies_safe():
"""Keep inline <script>/<style> bodies out of easywidgets' autoescaping.
- drop this once requirements pin easywidgets >= 0.5.
+
+ easywidgets 0.4.x renders JSScript/CSSScript through a bare ``{{widget.text}}``. That was
+ harmless while its jinja env had autoescaping off, but _make_core_app turns it on, which would
+ escape every inline script and style body in the site (``&&`` -> ``&&`` and so on).
+ Script bodies are developer-supplied markup by definition, so mark them safe. 0.5 does this
+ itself; drop this once requirements pin easywidgets >= 0.5.
"""
ew.jinja2_ew.JSScript.WidgetClass.template = ew.jinja2_ew.Snippet(
'<script type="text/javascript">{{widget.text|safe}}</script>', 'jinja2')
@@ -201,7 +173,6 @@ def _make_core_app(root, global_conf: dict, **app_conf):
app = SSLMiddleware(app, app_conf.get('no_redirect.pattern'),
app_conf.get('force_ssl.pattern'))
_mark_ew_script_bodies_safe()
- _bound_ew_resource_cache(asint(app_conf.get('ew.max_cache_bytes', 64 * 1024 * 1024)))
# Setup resource manager, widget context SOP
app = ew.WidgetMiddleware(
app,
@@ -212,10 +183,16 @@ def _make_core_app(root, global_conf: dict, **app_conf):
extra_headers=ast.literal_eval(app_conf.get('ew.extra_headers', '[]')),
cache_max_age=asint(app_conf.get('ew.cache_header_seconds', 60*60*24*365)),
- # settings to pass through to a second jinja Environment for EW core widgets
+ # settings to pass through to jinja Environment for EW core widgets
# these are for the easywidgets' own [easy_widgets.engines] entry point
# (the Allura [easy_widgets.engines] entry point is named "jinja" (not jinja2) but it doesn't need
# any settings since it is a class that uses the same jinja env as the rest of allura)
+ # NB: this is a *second* jinja environment, separate from the one in
+ # AlluraJinjaRenderer. Widgets render through this one and their output
+ # is spliced into our own templates as Markup, so it does not inherit
+ # their autoescaping -- it has to be turned on here too. Recent
+ # easywidgets defaults it on; set it explicitly so an older pin can't
+ # silently render widgets unescaped.
**{
'jinja2.auto_reload': asbool(config['auto_reload_templates']),
'jinja2.autoescape': True,
@@ -223,11 +200,6 @@ def _make_core_app(root, global_conf: dict, **app_conf):
'jinja2.cache_size': asint(config.get('jinja_cache_size', -1)),
}
)
- # Refuse oversized _slim requests before WidgetMiddleware can concatenate them
- app = EwSlimRequestLimitMiddleware(
- app,
- script_name=app_conf.get('ew.script_name', '/_ew_resources/'),
- max_hrefs=asint(app_conf.get('ew.max_slim_hrefs', 100)))
# Handle static files (by tool)
app = StaticFilesMiddleware(app, app_conf.get('static.script_name'))
# Handle setup and flushing of Ming ORM sessions
diff --git a/Allura/allura/lib/custom_middleware.py b/Allura/allura/lib/custom_middleware.py
index 5c8f427cc..183356f35 100644
--- a/Allura/allura/lib/custom_middleware.py
+++ b/Allura/allura/lib/custom_middleware.py
@@ -18,7 +18,6 @@
import os
import re
import logging
-from urllib.parse import parse_qs
import tg
from paste import fileapp
@@ -658,33 +657,3 @@ def __call__(self, environ, start_response):
newstatus, headers, app_iter, exc_info = _call_wsgi_application(self.app, new_environ)
start_response(status, headers, exc_info)
return app_iter
-
-
-class EwSlimRequestLimitMiddleware:
- """Cap how many files one /_ew_resources/_slim/ request may concatenate.
-
- easywidgets 0.4.x joins as many files as the query string names, with no bound, and its
- WidgetMiddleware answers that path before the wrapped app sees it -- so an unauthenticated
- request can name one file hundreds of times and get back a response hundreds of times its own
- size, which is then cached for the life of the process. Naming Allura's largest registered
- resource 500 times turns a 9 KB query string into a 155 MB response.
-
- This has to sit *outside* WidgetMiddleware so oversized requests never reach it. easywidgets
- 0.5 enforces the same limit itself via max_slim_hrefs; drop this once the pin moves.
- """
-
- def __init__(self, app, script_name='/_ew_resources/', max_hrefs=100):
- self.app = app
- self.slim_prefix = script_name + '_slim/'
- self.max_hrefs = max_hrefs
-
- def __call__(self, environ, start_response):
- if environ.get('PATH_INFO', '').startswith(self.slim_prefix):
- href = parse_qs(environ.get('QUERY_STRING', '')).get('href', [''])[0]
- count = len([h for h in href.split(';') if h])
- if count > self.max_hrefs:
- log.warning('Refusing _slim request for %s files (max %s) from %s',
- count, self.max_hrefs, environ.get('REMOTE_ADDR'))
- resp = exc.HTTPBadRequest('Too many resources requested')
- return resp(environ, start_response)
- return self.app(environ, start_response)
diff --git a/Allura/allura/lib/widgets/auth_widgets.py b/Allura/allura/lib/widgets/auth_widgets.py
index ef863229f..38efc5df5 100644
--- a/Allura/allura/lib/widgets/auth_widgets.py
+++ b/Allura/allura/lib/widgets/auth_widgets.py
@@ -29,12 +29,16 @@
from allura.lib import plugin
+# Emits already-rendered markup as-is. The antispam honeypot fields arrive as
+# Markup, so passing them as *template source* is both pointless and risky --
+# a '$' in the content would be read as a core-ew substitution. Render them as
+# data instead.
+_raw_html = ew.Snippet('{{widget.text|safe}}', 'jinja2')
+
+
class LoginForm(ForgeForm):
submit_text = 'Login'
style = 'wide'
- # The antispam honeypot fields arrive as Markup but it's better to
- # render them as data.
- _raw_html = ew.Snippet('{{widget.text|safe}}', 'jinja2')
@property
def fields(self):
@@ -65,7 +69,7 @@ def fields(self):
for fld in g.antispam.extra_fields():
fields.append(
- ew_core.Widget(template=self._raw_html, text=fld))
+ ew_core.Widget(template=_raw_html, text=fld))
return fields
diff --git a/Allura/allura/lib/widgets/forms.py b/Allura/allura/lib/widgets/forms.py
index a3a3fce50..de6eba00f 100644
--- a/Allura/allura/lib/widgets/forms.py
+++ b/Allura/allura/lib/widgets/forms.py
@@ -44,6 +44,8 @@
class _HTMLExplanation(ew.InputField):
+ # text is developer-supplied markup (never user input); |safe keeps
+ # easywidgets' autoescaping from mangling it
template = ew.Snippet(
'''<label class="grid-4"> </label>
<div class="grid-14" style="margin:2px;">{{widget.text|safe}}</div>
diff --git a/ForgeTracker/forgetracker/widgets/ticket_form.py b/ForgeTracker/forgetracker/widgets/ticket_form.py
index 30b6f8fea..6a3e59368 100644
--- a/ForgeTracker/forgetracker/widgets/ticket_form.py
+++ b/ForgeTracker/forgetracker/widgets/ticket_form.py
@@ -240,12 +240,12 @@ class MilestoneField(ew.SingleSelectField):
'class':css_class},
attrs)}}>
{% for o in open_milestones %}
- <option{% if o.selected%} selected{% endif %} value="{{o.html_value|e}}">{{o.label}}</option>
+ <option{% if o.selected%} selected{% endif %} value="{{o.html_value|e}}">{{o.label|e}}</option>
{% endfor %}
{% if closed_milestones %}
<optgroup label="Closed">
{% for o in closed_milestones %}
- <option{% if o.selected%} selected{% endif %} value="{{o.html_value|e}}">{{o.label}}</option>
+ <option{% if o.selected%} selected{% endif %} value="{{o.html_value|e}}">{{o.label|e}}</option>
{% endfor %}
</optgroup>
{% endif %}