Skip to content

Commit fb9be92

Browse files
fix(website): escape vulnerability IDs in hierarchy display (#5228)
## Summary `construct_hierarchy_string()` in `gcp/website/frontend_handlers.py` builds the upstream/downstream hierarchy HTML for vulnerability pages by concatenating raw `vuln_id` strings into `<li>` and `<a href=...>` fragments. The resulting string is rendered with Jinja2's `| safe` filter in `vulnerability.html`, so any markup characters that reach this code path are emitted verbatim. This PR wraps each id with `markupsafe.escape` and assembles the output as `markupsafe.Markup`, so ids drawn from source records are always HTML-escaped in the rendered hierarchy — matching how the rest of the template handles data-derived text. ## Changes - `gcp/website/frontend_handlers.py` — `construct_hierarchy_string` now returns `Markup`; each `vuln_id` is passed through `escape()` before being spliced into the `<li>` / `<a>` fragments, using `Markup(...).format(...)` so nested `Markup` fragments compose safely. - `gcp/website/frontend_handlers_test.py` — new `ConstructHierarchyStringTest` with three cases: - a known id containing `<script>` — verified not present verbatim, present as `&lt;script&gt;` - an unknown id containing `"><img ...>` — verified escaped - a plain `CVE-2024-0001` — verified the anchor still renders identically to before ## Testing - `python3 -m unittest frontend_handlers_test.ConstructHierarchyStringTest` — passes locally against the patched function. - Existing hierarchy rendering is byte-identical for ids containing only `[A-Za-z0-9:._-]`, so no change is expected for any well-formed OSV id. --------- Co-authored-by: TristanInSec <tristan.mtn@gmail.com>
1 parent eeb4e82 commit fb9be92

2 files changed

Lines changed: 53 additions & 12 deletions

File tree

gcp/website/frontend_handlers.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@ class ComputedHierarchy(NamedTuple):
11031103

11041104

11051105
def construct_hierarchy_string(target_bug_id: str, hierarchy: ComputedHierarchy,
1106-
known_ids: set[str]) -> str:
1106+
known_ids: set[str]) -> Markup:
11071107
"""Constructs a hierarchy string for display.
11081108
11091109
Args:
@@ -1112,9 +1112,12 @@ def construct_hierarchy_string(target_bug_id: str, hierarchy: ComputedHierarchy,
11121112
known_ids: A set of bug IDs that are known to exist (for link generation).
11131113
11141114
Returns:
1115-
A string representing the hierarchy for display by the frontend.
1115+
A Markup string representing the hierarchy for display by the frontend.
1116+
Vulnerability IDs are HTML-escaped so that values originating from
1117+
source records cannot inject markup when the template renders the
1118+
result.
11161119
"""
1117-
output_lines = []
1120+
output_lines: list[Markup] = []
11181121
root_nodes = hierarchy.root_nodes
11191122
graph = hierarchy.graph
11201123

@@ -1126,28 +1129,29 @@ def print_subtree(vuln_id: str) -> None:
11261129
vuln_id (str): The starting vuln_id for printing the subtree.
11271130
"""
11281131
if vuln_id != target_bug_id:
1132+
escaped_id = escape(vuln_id)
11291133
if vuln_id in known_ids:
1130-
output_lines.append("<li><a href=\"/vulnerability/" + vuln_id + "\">" +
1131-
vuln_id + " </a></li>")
1134+
output_lines.append(
1135+
Markup('<li><a href="/vulnerability/{0}">{0} </a></li>').format(
1136+
escaped_id))
11321137
else:
1133-
output_lines.append("<li>" + vuln_id + "</li>")
1138+
output_lines.append(Markup('<li>{0}</li>').format(escaped_id))
11341139

11351140
if vuln_id in graph:
11361141
sorted_children = sorted(graph[vuln_id])
11371142
for child in sorted_children:
11381143
if child != target_bug_id:
1139-
output_lines.append("<ul class=\"substream\">")
1144+
output_lines.append(Markup('<ul class="substream">'))
11401145
print_subtree(child)
1141-
output_lines.append("</ul>")
1146+
output_lines.append(Markup('</ul>'))
11421147

11431148
sorted_root_nodes = sorted(root_nodes)
11441149
for root in sorted_root_nodes:
1145-
output_lines.append("<ul class=\"aliases\">")
1150+
output_lines.append(Markup('<ul class="aliases">'))
11461151
print_subtree(root)
1147-
output_lines.append("</ul>")
1152+
output_lines.append(Markup('</ul>'))
11481153

1149-
final_string = "".join(output_lines)
1150-
return final_string
1154+
return Markup('').join(output_lines)
11511155

11521156

11531157
def reverse_tree(graph: dict[str, set[str]]) -> dict[str, set[str]]:

gcp/website/frontend_handlers_test.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,43 @@ def test_escapes_xss(self):
157157
self.assertIn('&lt;script&gt;', result)
158158

159159

160+
class ConstructHierarchyStringTest(unittest.TestCase):
161+
"""Tests for frontend_handlers.construct_hierarchy_string."""
162+
163+
def _hierarchy(self, roots, graph):
164+
return frontend_handlers.ComputedHierarchy(
165+
root_nodes=set(roots), graph={
166+
k: set(v) for k, v in graph.items()
167+
})
168+
169+
def test_escapes_known_id(self):
170+
"""IDs that match known_ids must be HTML-escaped inside the <a> tag."""
171+
payload = '<script>alert(1)</script>'
172+
hierarchy = self._hierarchy([payload], {})
173+
result = frontend_handlers.construct_hierarchy_string(
174+
'CVE-TARGET', hierarchy, {payload})
175+
self.assertNotIn('<script>', result)
176+
self.assertIn('&lt;script&gt;', result)
177+
178+
def test_escapes_unknown_id(self):
179+
"""IDs that are not known still appear in a <li> and must be escaped."""
180+
payload = '"><img src=x onerror=alert(1)>'
181+
hierarchy = self._hierarchy([payload], {})
182+
result = frontend_handlers.construct_hierarchy_string(
183+
'CVE-TARGET', hierarchy, set())
184+
self.assertNotIn('<img', result)
185+
self.assertIn('&lt;img', result)
186+
self.assertIn('&#34;', result)
187+
188+
def test_plain_id_rendered_as_link(self):
189+
"""Well-formed IDs still render as a normal anchor."""
190+
hierarchy = self._hierarchy(['CVE-2024-0001'], {})
191+
result = frontend_handlers.construct_hierarchy_string(
192+
'CVE-TARGET', hierarchy, {'CVE-2024-0001'})
193+
self.assertIn('<a href="/vulnerability/CVE-2024-0001">', result)
194+
self.assertIn('CVE-2024-0001 </a>', result)
195+
196+
160197
def setUpModule():
161198
"""Set up the test module."""
162199
# Start the emulator BEFORE creating the ndb client

0 commit comments

Comments
 (0)