Output of diff not preserving escaped HTML characters

cyn8 commented 1 year ago

Step 0: Describe your environment

diff2html version: Latest from CDN
Using diff2html directly or using diff2html-ui helper: diff2html-ui
Extra flags: { drawFileList: true, matching: 'lines', highlight: true }

Step 1: Describe the problem:

Because some of the diffs I have use HTML metacharacters (<>, etc), I run the generated diff through the PHP function htmlspecialchars() before passing it to diff2html-ui, in order prevent injection by malicious code inside diffs.

Unfortunately, by doing so, the actual < and > characters are not rendered, instead I see < and > in the browser, and by viewing the rendered HTML, it seems those characters are rendered as follows (snippet):

<span class="d2h-code-line-ctn hljs plaintext">&amp;lt;div class=&amp;quot;col-xl-6&amp;quot;&amp;gt;</span>

I have tried not using htmlspecialchars() to display the diff, but then the page breaks as sometimes the diff will include things like </script> etc.

Steps to reproduce:

Generate diff from two HTML files

Display using the following:

document.addEventListener('DOMContentLoaded', () => {
    const diffString = `<?php print(htmlspecialchars($diff)); ?>`;
    const targetElement = document.getElementById('myDiffElement');
    const configuration = { drawFileList: true, matching: 'lines', highlight: true };
    const diff2htmlUi = new Diff2HtmlUI(targetElement, diffString, configuration);
    diff2htmlUi.draw();
    diff2htmlUi.highlightCode();
});

diff example:

index e66d84f..d7b39ea 100644
@@ -98,12 +98,12 @@
 &lt;div class=&quot;row&quot;&gt;
 &lt;div class=&quot;col-xl-6&quot;&gt;
 &lt;ul&gt;
-&lt;li&gt;General Questions/Comments&lt;br&gt;&lt;span style=&quot;color: #a0a0a0;&quot;&gt;&lt;a href=&quot;/cdn-cgi/l/email-protection&quot; class=&quot;__cf_email__&quot; data-cfemail=&quot;f49c9198989bb4989d8091879197da979b&quot;&gt;[email&amp;#160;protected]&lt;/a&gt;&lt;/span&gt;&lt;/li&gt;
+&lt;li&gt;General Questions/Comments&lt;br&gt;&lt;span style=&quot;color: #a0a0a0;&quot;&gt;&lt;a href=&quot;/cdn-cgi/l/email-protection&quot; class=&quot;__cf_email__&quot; data-cfemail=&quot;1078757c7c7f507c7964756375733e737f&quot;&gt;[email&amp;#160;protected]&lt;/a&gt;&lt;/span&gt;&lt;/li&gt;
 &lt;/ul&gt;
 &lt;/div&gt;
 &lt;div class=&quot;col-xl-6&quot;&gt;
 &lt;ul&gt;
-&lt;li&gt;After-sales/Technical Support&lt;br&gt;&lt;span style=&quot;color: #a0a0a0;&quot;&gt;&lt;a href=&quot;/cdn-cgi/l/email-protection&quot; class=&quot;__cf_email__&quot; data-cfemail=&quot;74070104041b060034181d00110711175a171b&quot;&gt;[email&amp;#160;protected]&lt;/a&gt;&lt;/span&gt;&lt;/li&gt;
+&lt;li&gt;After-sales/Technical Support&lt;br&gt;&lt;span style=&quot;color: #a0a0a0;&quot;&gt;&lt;a href=&quot;/cdn-cgi/l/email-protection&quot; class=&quot;__cf_email__&quot; data-cfemail=&quot;4a393f3a3a25383e0a26233e2f392f29642925&quot;&gt;[email&amp;#160;protected]&lt;/a&gt;&lt;/span&gt;&lt;/li&gt;
 &lt;/ul&gt;
 &lt;/div&gt;
 &lt;/div&gt;
@@ -116,5 +116,5 @@
 &lt;script src=&quot;/assets/scripts/gsap.min.js&quot;&gt;&lt;/script&gt;
 &lt;script src=&quot;/assets/scripts/ScrollTrigger.min.js&quot;&gt;&lt;/script&gt;
 &lt;script src=&quot;/assets/scripts/scripts.js&quot;&gt;&lt;/script&gt;
-&lt;script&gt;(function(){var js = &quot;window[&#039;__CF$cv$params&#039;]={r:&#039;7864be10ba44a974&#039;,m:&#039;X08gmzxgJnVxp5.yySJx_c10B3QMfcRrSa.x2OXSK7M-1673178908-0-AbwKUmB+sIxi8k2gIYbbMjiRe9tar5ulYusc4FYDgnvhRrdF4NpFOkEf4tsosLqyellSvV8PRlQ6pyaMBrQ4/dONiHi3JJibZaLR/TNg8doht9zKgdXFcYPwXpxlTDW7SA==&#039;,s:[0xdbcd5ad0cf,0x62f5d1dccb],u:&#039;/cdn-cgi/challenge-platform/h/b&#039;};var now=Date.now()/1000,offset=14400,ts=&#039;&#039;+(Math.floor(now)-Math.floor(now%offset)),_cpo=document.createElement(&#039;script&#039;);_cpo.nonce=&#039;&#039;,_cpo.src=&#039;/cdn-cgi/challenge-platform/h/b/scripts/alpha/invisible.js?ts=&#039;+ts,document.getElementsByTagName(&#039;head&#039;)[0].appendChild(_cpo);&quot;;var _0xh = document.createElement(&#039;iframe&#039;);_0xh.height = 1;_0xh.width = 1;_0xh.style.position = &#039;absolute&#039;;_0xh.style.top = 0;_0xh.style.left = 0;_0xh.style.border = &#039;none&#039;;_0xh.style.visibility = &#039;hidden&#039;;document.body.appendChild(_0xh);function handler() {var _0xi = _0xh.contentDocument || _0xh.contentWindow.document;if (_0xi) {var _0xj = _0xi.createElement(&#039;script&#039;);_0xj.nonce = &#039;&#039;;_0xj.innerHTML = js;_0xi.getElementsByTagName(&#039;head&#039;)[0].appendChild(_0xj);}}if (document.readyState !== &#039;loading&#039;) {handler();} else if (window.addEventListener) {document.addEventListener(&#039;DOMContentLoaded&#039;, handler);} else {var prev = document.onreadystatechange || function () {};document.onreadystatechange = function (e) {prev(e);if (document.readyState !== &#039;loading&#039;) {document.onreadystatechange = prev;handler();}};}})();&lt;/script&gt;&lt;/body&gt;
+&lt;script&gt;(function(){var js = &quot;window[&#039;__CF$cv$params&#039;]={r:&#039;787b3ed179a3a813&#039;,m:&#039;XIMKBoRKBij0zGsCFsLPKKxi7IZ0r7z2QeG887uWxMs-1673414868-0-AcbIgMblnqiXyuIv+gAn3H3OWbJpHq5zCy4JZ0AylFXpimnRLuScc0G0e9msVAKA8gblxfJ7pZzMpEhcnR+Tr/97VvcBmD6pGXWRCYUgr0ekEa1cWdyebj37pN6AC688Gg==&#039;,s:[0x3531a8363b,0x37ba8a663e],u:&#039;/cdn-cgi/challenge-platform/h/g&#039;};var now=Date.now()/1000,offset=14400,ts=&#039;&#039;+(Math.floor(now)-Math.floor(now%offset)),_cpo=document.createElement(&#039;script&#039;);_cpo.nonce=&#039;&#039;,_cpo.src=&#039;/cdn-cgi/challenge-platform/h/g/scripts/alpha/invisible.js?ts=&#039;+ts,document.getElementsByTagName(&#039;head&#039;)[0].appendChild(_cpo);&quot;;var _0xh = document.createElement(&#039;iframe&#039;);_0xh.height = 1;_0xh.width = 1;_0xh.style.position = &#039;absolute&#039;;_0xh.style.top = 0;_0xh.style.left = 0;_0xh.style.border = &#039;none&#039;;_0xh.style.visibility = &#039;hidden&#039;;document.body.appendChild(_0xh);function handler() {var _0xi = _0xh.contentDocument || _0xh.contentWindow.document;if (_0xi) {var _0xj = _0xi.createElement(&#039;script&#039;);_0xj.nonce = &#039;&#039;;_0xj.innerHTML = js;_0xi.getElementsByTagName(&#039;head&#039;)[0].appendChild(_0xj);}}if (document.readyState !== &#039;loading&#039;) {handler();} else if (window.addEventListener) {document.addEventListener(&#039;DOMContentLoaded&#039;, handler);} else {var prev = document.onreadystatechange || function () {};document.onreadystatechange = function (e) {prev(e);if (document.readyState !== &#039;loading&#039;) {document.onreadystatechange = prev;handler();}};}})();&lt;/script&gt;&lt;/body&gt;
 &lt;/html&gt;
\ No newline at end of file

Observed Results:

Escaped HTML character codes are displayed raw

Expected Results:

I expected < to be displayed as <, and vice versa.

Relevant Code:

see above (step 2)

rtfpessoa commented 1 year ago

diff2html handles escaping already so you should not need to escape anything before invoking it.

Can you provide an example to replicate the problem?

cyn8 commented 1 year ago

I ended up changing the code around so that the diff variable is filled with the contents of a separate request made in JS. I originally had just rendered the page with the unescaped contents within the <script> tag, which could lead to cross-site scripting if provided with a malicious diff.

I guess an option to enable/disable the escaping provided by diff2html would be nice, but there are other ways to solve the problem I should've looked into. Thanks for the quick reply.

rtfpessoa commented 1 year ago

Not sure I understand what you mean, but diff2html should not be vulnerable to any type of injection. If it is I would like to know about it a fix it. So if you were able to please provide an example.

rtfpessoa / diff2html