Skip to content

Commit 64312a4

Browse files
[1.6] Sanitize render_markdown() output with nh3 library (#5134)
* Fix GHSA-v4xv-795h-rv4h. * Renumber change fragments * Address review feedback * Test fix * Ruff * Review feedback * Update nautobot/utilities/forms/fields.py Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com> * Update nautobot/utilities/forms/fields.py --------- Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>
1 parent 1c3eea0 commit 64312a4

File tree

18 files changed

+221
-45
lines changed

18 files changed

+221
-45
lines changed

Diff for: changes/5134.added

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Enhanced Markdown-supporting fields (`comments`, `description`, Notes, Job log entries, etc.) to also permit the use of a limited subset of "safe" HTML tags and attributes.

Diff for: changes/5134.dependencies

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Added `nh3` HTML sanitization library as a dependency.

Diff for: changes/5134.security

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed an XSS vulnerability ([GHSA-v4xv-795h-rv4h](https://github.com/nautobot/nautobot/security/advisories/GHSA-v4xv-795h-rv4h)) in the `render_markdown()` utility function used to render comments, notes, job log entries, etc.

Diff for: nautobot/core/settings.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,8 @@
671671
],
672672
"SUPPORT_MESSAGE": [
673673
"",
674-
"Help message to include on 4xx and 5xx error pages. Markdown is supported.\n"
674+
"Help message to include on 4xx and 5xx error pages. "
675+
"Markdown is supported, as are some HTML tags and attributes.\n"
675676
"If unspecified, instructions to join Network to Code's Slack community will be provided.",
676677
],
677678
}

Diff for: nautobot/core/tests/test_views.py

+7-7
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,9 @@ def test_404_default_support_message(self):
370370
self.assertContains(response, "Network to Code", status_code=404)
371371
response_content = response.content.decode(response.charset)
372372
self.assertInHTML(
373-
"If further assistance is required, please join the <code>#nautobot</code> channel "
374-
'on <a href="https://slack.networktocode.com/">Network to Code\'s Slack community</a> '
375-
"and post your question.",
373+
"If further assistance is required, please join the <code>#nautobot</code> channel on "
374+
'<a href="https://slack.networktocode.com/" rel="noopener noreferrer">Network to Code\'s '
375+
"Slack community</a> and post your question.",
376376
response_content,
377377
)
378378

@@ -396,16 +396,16 @@ def test_500_default_support_message(self, mock_get):
396396
self.assertContains(response, "Network to Code", status_code=500)
397397
response_content = response.content.decode(response.charset)
398398
self.assertInHTML(
399-
"If further assistance is required, please join the <code>#nautobot</code> channel "
400-
'on <a href="https://slack.networktocode.com/">Network to Code\'s Slack community</a> '
401-
"and post your question.",
399+
"If further assistance is required, please join the <code>#nautobot</code> channel on "
400+
'<a href="https://slack.networktocode.com/" rel="noopener noreferrer">Network to Code\'s '
401+
"Slack community</a> and post your question.",
402402
response_content,
403403
)
404404

405405
@override_settings(DEBUG=False, SUPPORT_MESSAGE="Hello world!")
406406
@mock.patch("nautobot.core.views.HomeView.get", side_effect=Exception)
407407
def test_500_custom_support_message(self, mock_get):
408-
"""Nautobot's custom 500 page should be used and should include a default support message."""
408+
"""Nautobot's custom 500 page should be used and should include a custom support message if defined."""
409409
url = reverse("home")
410410
with self.assertTemplateUsed("500.html"):
411411
self.client.raise_request_exception = False

Diff for: nautobot/docs/additional-features/jobs.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ This is the human-friendly name of your job, as will be displayed in the Nautobo
9090
#### `description`
9191

9292
An optional human-friendly description of what this job does.
93-
This can accept either plain text or Markdown-formatted text. It can also be multiple lines:
93+
This can accept either plain text, Markdown-formatted text, or [a limited subset of HTML](template-filters.md#render_markdown). It can also be multiple lines:
9494

9595
```python
9696
class ExampleJob(Job):
@@ -466,7 +466,7 @@ Messages recorded with `log()` or `log_debug()` will appear in a job's results b
466466

467467
It is advised to log a message for each object that is evaluated so that the results will reflect how many objects are being manipulated or reported on.
468468

469-
Markdown rendering is supported for log messages.
469+
Markdown rendering is supported for log messages, as well as [a limited subset of HTML](template-filters.md#render_markdown).
470470

471471
+/- 1.3.4
472472
As a security measure, the `message` passed to any of these methods will be passed through the `nautobot.utilities.logging.sanitize()` function in an attempt to strip out information such as usernames/passwords that should not be saved to the logs. This is of course best-effort only, and Job authors should take pains to ensure that such information is not passed to the logging APIs in the first place. The set of redaction rules used by the `sanitize()` function can be configured as [settings.SANITIZER_PATTERNS](../configuration/optional-settings.md#sanitizer_patterns).

Diff for: nautobot/docs/additional-features/template-filters.md

+75-1
Original file line numberDiff line numberDiff line change
@@ -207,12 +207,86 @@ Render a dictionary as formatted JSON.
207207

208208
### render_markdown
209209

210-
Render text as Markdown.
210+
Render and sanitize Markdown text into HTML. A limited subset of HTML tags and attributes are permitted in the text as well; non-permitted HTML will be stripped from the output for security.
211211

212212
```django
213213
{{ text | render_markdown }}
214214
```
215215

216+
#### Permitted HTML Tags and Attributes
217+
218+
+++ 1.6.10
219+
220+
The set of permitted HTML tags is defined in `nautobot.core.constants.HTML_ALLOWED_TAGS`, and their permitted attributes are defined in `nautobot.core.constants.HTML_ALLOWED_ATTRIBUTES`. As of Nautobot 1.6.10 the following are permitted:
221+
222+
??? info "Full list of HTML tags and attributes"
223+
| Tag | Attributes |
224+
| -------------- | -------------------------------------------------------------------- |
225+
| `<a>` | `href`, `hreflang` |
226+
| `<abbr>` | |
227+
| `<acronym>` | |
228+
| `<b>` | |
229+
| `<bdi>` | |
230+
| `<bdo>` | `dir` |
231+
| `<blockquote>` | `cite` |
232+
| `<br>` | |
233+
| `<caption>` | |
234+
| `<center>` | |
235+
| `<cite>` | |
236+
| `<code>` | |
237+
| `<col>` | `align`, `char`, `charoff`, `span` |
238+
| `<colgroup>` | `align`, `char`, `charoff`, `span` |
239+
| `<dd>` | |
240+
| `<del>` | `cite`, `datetime` |
241+
| `<details>` | |
242+
| `<div>` | |
243+
| `<dl>` | |
244+
| `<dt>` | |
245+
| `<em>` | |
246+
| `<h1>` | |
247+
| `<h2>` | |
248+
| `<h3>` | |
249+
| `<h4>` | |
250+
| `<h5>` | |
251+
| `<h6>` | |
252+
| `<hgroup>` | |
253+
| `<hr>` | `align`, `size`, `width` |
254+
| `<i>` | |
255+
| `<img>` | `align`, `alt`, `height`, `src`, `width` |
256+
| `<ins>` | `cite`, `datetime` |
257+
| `<kbd>` | |
258+
| `<li>` | |
259+
| `<mark>` | |
260+
| `<ol>` | `start` |
261+
| `<p>` | |
262+
| `<pre>` | |
263+
| `<q>` | `cite` |
264+
| `<rp>` | |
265+
| `<rt>` | |
266+
| `<rtc>` | |
267+
| `<ruby>` | |
268+
| `<s>` | |
269+
| `<samp>` | |
270+
| `<small>` | |
271+
| `<span>` | |
272+
| `<strike>` | |
273+
| `<strong>` | |
274+
| `<sub>` | |
275+
| `<summary>` | |
276+
| `<sup>` | |
277+
| `<table>` | `align`, `char`, `charoff`, `summary` |
278+
| `<tbody>` | `align`, `char`, `charoff` |
279+
| `<td>` | `align`, `char`, `charoff`, `colspan`, `headers`, `rowspan` |
280+
| `<th>` | `align`, `char`, `charoff`, `colspan`, `headers`, `rowspan`, `scope` |
281+
| `<thead>` | `align`, `char`, `charoff` |
282+
| `<time>` | |
283+
| `<tr>` | `align`, `char`, `charoff` |
284+
| `<tt>` | |
285+
| `<u>` | |
286+
| `<ul>` | |
287+
| `<var>` | |
288+
| `<wbr>` | |
289+
216290
### render_yaml
217291

218292
Render a dictionary as formatted YAML.

Diff for: nautobot/docs/configuration/optional-settings.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ If set to `False`, unknown/unrecognized filter parameters will be discarded and
901901

902902
Default: `""`
903903

904-
A message to include on error pages (status code 403, 404, 500, etc.) when an error occurs. You can configure this to direct users to the appropriate contact(s) within your organization that provide support for Nautobot. Markdown formatting is supported within this message (raw HTML is not).
904+
A message to include on error pages (status code 403, 404, 500, etc.) when an error occurs. You can configure this to direct users to the appropriate contact(s) within your organization that provide support for Nautobot. Markdown formatting is supported within this message, as well as [a limited subset of HTML](../additional-features/template-filters.md#render_markdown).
905905

906906
If unset, the default message that will appear is `If further assistance is required, please join the #nautobot channel on [Network to Code's Slack community](https://slack.networktocode.com) and post your question.`
907907

Diff for: nautobot/docs/models/extras/note.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44

55
Notes provide a place for you to store notes or general information on an object, such as a Device, that may not require a specific field for. This could be a note on a recent upgrade, a warning about a problematic device, or the reason the Rack was marked with the Status `Retired`.
66

7-
The note field supports [Markdown Basic Syntax](https://www.markdownguide.org/cheat-sheet/#basic-syntax).
7+
The note field supports [Markdown Basic Syntax](https://www.markdownguide.org/cheat-sheet/#basic-syntax) as well as [a limited subset of HTML](../../additional-features/template-filters.md#render_markdown).

Diff for: nautobot/extras/forms/forms.py

+9-5
Original file line numberDiff line numberDiff line change
@@ -360,18 +360,22 @@ class ConfigContextSchemaFilterForm(BootstrapMixin, forms.Form):
360360
)
361361

362362

363+
class CustomFieldDescriptionField(CommentField):
364+
@property
365+
def default_helptext(self):
366+
return "Also used as the help text when editing models using this custom field.<br>" + super().default_helptext
367+
368+
363369
class CustomFieldForm(BootstrapMixin, forms.ModelForm):
364370
label = forms.CharField(required=True, max_length=50, help_text="Name of the field as displayed to users.")
365371
slug = SlugField(
366372
max_length=50,
367373
slug_source="label",
368374
help_text="Internal name of this field. Please use underscores rather than dashes.",
369375
)
370-
description = forms.CharField(
376+
description = CustomFieldDescriptionField(
377+
label="Description",
371378
required=False,
372-
help_text="Also used as the help text when editing models using this custom field.<br>"
373-
'<a href="https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet" target="_blank">'
374-
"Markdown</a> syntax is supported.",
375379
)
376380
content_types = MultipleContentTypeField(
377381
feature="custom_fields", help_text="The object(s) to which this field applies."
@@ -1083,7 +1087,7 @@ class JobButtonFilterForm(BootstrapMixin, forms.Form):
10831087

10841088

10851089
class NoteForm(BootstrapMixin, forms.ModelForm):
1086-
note = CommentField
1090+
note = CommentField()
10871091

10881092
class Meta:
10891093
model = Note

Diff for: nautobot/extras/models/jobs.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,10 @@ class Job(PrimaryModel):
140140
help_text="Human-readable name of this job",
141141
db_index=True,
142142
)
143-
description = models.TextField(blank=True, help_text="Markdown formatting is supported")
143+
description = models.TextField(
144+
blank=True,
145+
help_text="Markdown formatting and a limited subset of HTML are supported",
146+
)
144147

145148
# Control flags
146149
installed = models.BooleanField(

Diff for: nautobot/utilities/constants.py

+33
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
from copy import deepcopy
2+
3+
import nh3
4+
15
#
26
# Filter lookup expressions
37
#
@@ -29,6 +33,35 @@
2933
FILTER_NEGATION_LOOKUP_MAP = {"n": "exact"}
3034

3135

36+
#
37+
# User input sanitization
38+
#
39+
40+
# Subset of the HTML tags allowed by default by ammonia:
41+
# https://github.com/rust-ammonia/ammonia/blob/master/src/lib.rs
42+
HTML_ALLOWED_TAGS = nh3.ALLOWED_TAGS - {
43+
# no image maps at present
44+
"area",
45+
"map",
46+
# no document-level markup at present
47+
"article",
48+
"aside",
49+
"footer",
50+
"header",
51+
"nav",
52+
# miscellaneous out-of-scope for now
53+
"data",
54+
"dfn",
55+
"figcaption",
56+
"figure",
57+
}
58+
59+
# Variant of the HTML attributes allowed by default by ammonia:
60+
# https://github.com/rust-ammonia/ammonia/blob/master/src/lib.rs
61+
# at present we just copy nh3.ALLOWED_ATTRIBUTES but we can modify this later as desired and appropriate
62+
HTML_ALLOWED_ATTRIBUTES = deepcopy(nh3.ALLOWED_ATTRIBUTES)
63+
64+
3265
#
3366
# HTTP Request META safe copy
3467
#

Diff for: nautobot/utilities/forms/fields.py

+12-6
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist, ValidationError
1313
from django.db.models import Q
1414
from django.forms.fields import BoundField, JSONField as _JSONField, InvalidJSONInput
15+
from django.templatetags.static import static
1516
from django.urls import reverse
17+
from django.utils.html import format_html
1618

1719
from nautobot.extras.utils import FeatureQuery
1820
from nautobot.utilities.choices import unpack_grouped_choices
@@ -391,12 +393,16 @@ class CommentField(forms.CharField):
391393

392394
widget = forms.Textarea
393395
default_label = ""
394-
# TODO: Port Markdown cheat sheet to internal documentation
395-
default_helptext = (
396-
'<i class="mdi mdi-information-outline"></i> '
397-
'<a href="https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet" target="_blank">'
398-
"Markdown</a> syntax is supported"
399-
)
396+
397+
@property
398+
def default_helptext(self):
399+
# TODO: Port Markdown cheat sheet to internal documentation
400+
return format_html(
401+
'<i class="mdi mdi-information-outline"></i> '
402+
'<a href="https://www.markdownguide.org/cheat-sheet/#basic-syntax" rel="noopener noreferrer">Markdown</a> '
403+
'syntax is supported, as well as <a href="{}#render_markdown">a limited subset of HTML</a>.',
404+
static("docs/additional-features/template-filters.html"),
405+
)
400406

401407
def __init__(self, *args, **kwargs):
402408
required = kwargs.pop("required", False)

Diff for: nautobot/utilities/logging.py

+13
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
import re
55

66
from django.conf import settings
7+
import nh3
8+
9+
from nautobot.utilities import constants
710

811

912
logger = logging.getLogger(__name__)
@@ -27,3 +30,13 @@ def sanitize(string, replacement="(redacted)"):
2730
logger.error('Error in string sanitization using "%s"', sanitizer)
2831

2932
return string
33+
34+
35+
def clean_html(html):
36+
"""Use nh3/ammonia to strip out all HTML tags and attributes except those explicitly permitted."""
37+
return nh3.clean(
38+
html,
39+
tags=constants.HTML_ALLOWED_TAGS,
40+
attributes=constants.HTML_ALLOWED_ATTRIBUTES,
41+
url_schemes=set(settings.ALLOWED_URL_SCHEMES),
42+
)

Diff for: nautobot/utilities/templatetags/helpers.py

+6-10
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
from django.contrib.staticfiles.finders import find
99
from django.templatetags.static import static, StaticNode
1010
from django.urls import NoReverseMatch, reverse
11-
from django.utils.html import format_html, strip_tags
11+
from django.utils.html import format_html
1212
from django.utils.safestring import mark_safe
1313
from markdown import markdown
1414
from django_jinja import library
1515

1616
from nautobot.utilities.config import get_settings_or_config
1717
from nautobot.utilities.forms import TableConfigForm
18+
from nautobot.utilities.logging import clean_html
1819
from nautobot.utilities.utils import foreground_color, get_route_for_model, UtilizationData
1920

2021
HTML_TRUE = mark_safe('<span class="text-success"><i class="mdi mdi-check-bold" title="Yes"></i></span>') # noqa: S308
@@ -162,18 +163,13 @@ def render_markdown(value):
162163
Example:
163164
{{ text | render_markdown }}
164165
"""
165-
# Strip HTML tags
166-
value = strip_tags(value)
167-
168-
# Sanitize Markdown links
169-
schemes = "|".join(settings.ALLOWED_URL_SCHEMES)
170-
pattern = rf"\[(.+)\]\((?!({schemes})).*:(.+)\)"
171-
value = re.sub(pattern, "[\\1](\\3)", value, flags=re.IGNORECASE)
172-
173166
# Render Markdown
174167
html = markdown(value, extensions=["fenced_code", "tables"])
175168

176-
return mark_safe(html) # noqa: S308
169+
# Sanitize rendered HTML
170+
html = clean_html(html)
171+
172+
return mark_safe(html) # noqa: S308 # suspicious-mark-safe-usage, OK here since we sanitized the string earlier
177173

178174

179175
@library.filter()

0 commit comments

Comments
 (0)