Skip to content

Commit 83bf285

Browse files
authored
htmlreport: escape message and file name in generated pages (#8920)
AnnotateCodeFormatter.wrap put the error message into the annotated source page, and main put the file name into the index, stats and per-file pages, without html_escape, so text from the checked code such as `#error <img src=x onerror=alert(1)>` or a file named `<b>x.c` became live markup in the report; both now go through html_escape like the verbose text and the index message already do, and test_htmlreport.py has a test for it.
1 parent 5587463 commit 83bf285

2 files changed

Lines changed: 35 additions & 9 deletions

File tree

‎htmlreport/cppcheck-htmlreport‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -620,15 +620,15 @@ class AnnotateCodeFormatter(HtmlFormatter):
620620
# from actual message
621621
if error.get('verbose') and (error['verbose'] != error['msg']):
622622
index = t.rfind('\n')
623-
t = t[:index] + HTML_EXPANDABLE_INCONCLUSIVE % (error['msg'], html_escape(error['verbose'].replace("\\012", '\n'))) + t[index + 1:]
623+
t = t[:index] + HTML_EXPANDABLE_INCONCLUSIVE % (html_escape(error['msg']), html_escape(error['verbose'].replace("\\012", '\n'))) + t[index + 1:]
624624
else:
625-
t = t.replace('\n', HTML_INCONCLUSIVE % error['msg'])
625+
t = t.replace('\n', HTML_INCONCLUSIVE % html_escape(error['msg']))
626626
except KeyError:
627627
if error.get('verbose') and (error['verbose'] != error['msg']):
628628
index = t.rfind('\n')
629-
t = t[:index] + HTML_EXPANDABLE_ERROR % (error['msg'], html_escape(error['verbose'].replace("\\012", '\n'))) + t[index + 1:]
629+
t = t[:index] + HTML_EXPANDABLE_ERROR % (html_escape(error['msg']), html_escape(error['verbose'].replace("\\012", '\n'))) + t[index + 1:]
630630
else:
631-
t = t.replace('\n', HTML_ERROR % error['msg'])
631+
t = t.replace('\n', HTML_ERROR % html_escape(error['msg']))
632632

633633
line_no = line_no + 1
634634
yield i, t
@@ -824,7 +824,7 @@ def main() -> None:
824824
if is_remote:
825825
# Construct remote URL for GitHub/GitLab
826826
# tr_str() will use the actual line number, so we can just start with line 1
827-
remote_url = source_dir.rstrip('/') + '/' + filename + '#L1'
827+
remote_url = source_dir.rstrip('/') + '/' + html_escape(filename) + '#L1'
828828
files[filename] = {'errors': [], 'htmlfile': remote_url}
829829
else:
830830
files[filename] = {'errors': [], 'htmlfile': str(file_no) + '.html'}
@@ -900,10 +900,10 @@ def main() -> None:
900900
(options.title,
901901
htmlFormatter.get_style_defs('.highlight'),
902902
options.title,
903-
': ' + filename))
903+
': ' + html_escape(filename)))
904904
output_file.write(HTML_HEAD_END)
905905

906-
output_file.write(HTML_MENU % (filename.split('/')[-1]))
906+
output_file.write(HTML_MENU % (html_escape(filename.split('/')[-1])))
907907
for error in sorted(errors, key=lambda k: k['line']):
908908
output_file.write("<a href=\"%s#line-%d\"> %s %s</a>" % (data['htmlfile'], error['line'], error['id'], error['line']))
909909
output_file.write(HTML_MENU_END)
@@ -1006,7 +1006,7 @@ def main() -> None:
10061006
for filename, data in sorted(files.items()):
10071007
file_error = filename in decode_errors or filename.endswith('*')
10081008
is_file = filename != '' and not file_error
1009-
row_content = filename if file_error else "<a href=\"%s\">%s</a>" % (data['htmlfile'], filename)
1009+
row_content = html_escape(filename) if file_error else "<a href=\"%s\">%s</a>" % (data['htmlfile'], html_escape(filename))
10101010
htmlfile = data.get('htmlfile') if is_file else None
10111011

10121012
output_file.write("\n <tbody class=\"fileEntry\">")
@@ -1122,7 +1122,7 @@ def main() -> None:
11221122
if it == 0:
11231123
LENGTH = len(str(i[1])) # <- length of longest number, now get the difference and try to make other numbers align to it
11241124

1125-
stats_file.write("&#160;" * 3 + str(i[1]) + "&#160;" * (1 + LENGTH - len(str(i[1]))) + "<a href=\"" + files[i[0]]['htmlfile'] + "\"> " + i[0] + "</a><br>\n")
1125+
stats_file.write("&#160;" * 3 + str(i[1]) + "&#160;" * (1 + LENGTH - len(str(i[1]))) + "<a href=\"" + files[i[0]]['htmlfile'] + "\"> " + html_escape(i[0]) + "</a><br>\n")
11261126
it += 1
11271127
if it == 10: # print only the top 10
11281128
break

‎test/tools/htmlreport/test_htmlreport.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,32 @@ def testSeverityFilterBar(self):
114114
self.assertIn('onclick="toggleSeverity(this)"', report)
115115
output_directory.cleanup()
116116

117+
def testEscape(self):
118+
with tempfile.TemporaryDirectory() as source_directory:
119+
source_filename = os.path.join(source_directory, '<b>escape.c')
120+
with open(source_filename, 'w') as source_file:
121+
source_file.write('#error <b>escape</b>\n')
122+
123+
with runCheck(
124+
source_filename,
125+
xml_version='2'
126+
) as (report, output_directory):
127+
self.assertIn('&lt;b&gt;escape.c', report)
128+
self.assertNotIn('<b>', report)
129+
130+
with open(os.path.join(output_directory.name, '0.html')) as input_file:
131+
detail_contents = input_file.read()
132+
self.assertIn('&lt;b&gt;escape.c', detail_contents)
133+
self.assertIn('&lt;--- #error &lt;b&gt;escape&lt;/b&gt;', detail_contents)
134+
self.assertNotIn('<b>', detail_contents)
135+
136+
with open(os.path.join(output_directory.name, 'stats.html')) as input_file:
137+
stats_contents = input_file.read()
138+
self.assertIn('&lt;b&gt;escape.c', stats_contents)
139+
self.assertNotIn('<b>', stats_contents)
140+
141+
output_directory.cleanup()
142+
117143

118144
@contextlib.contextmanager
119145
def runCheck(source_filename=None, xml_version='1', xml_filename=None, checkers_filename=None):

0 commit comments

Comments
 (0)