SONAR-17201 Sanitize code before injecting it in DOM + UI fix

plugins/sonar-education-plugin/src/main/resources/org/sonar/education/2codeSnippets.html

@@ -1,45 +1,70 @@
-<p>An infinite loop is one that will never end while the program is running, i.e., you have to kill the program to get out of the loop. Whether it is
-    by meeting the loop’s end condition or via a <code>break</code>, every loop should have an end condition.</p>
-<h3>Known Limitations</h3>
-<ul>
-    <li> False positives: when <code>yield</code> is used - <a href="https://github.com/SonarSource/SonarJS/issues/674">Issue #674</a>. </li>
-    <li> False positives: when an exception is raised by a function invoked within the loop. </li>
-    <li> False negatives: when a loop condition is based on an element of an array or object. </li>
-</ul>
-<h2>Noncompliant Code Example</h2>
-<pre data-diff-id="example-1" data-diff-type="noncompliant">for (;;) {  // Noncompliant; end condition omitted
-  // ...
-}
+<p>This is an example of an attempt to run some code on our</p>
+<h3>Cross-site scripting (XSS) attack</h3>
 
-var j = 0;
-while (true) { // Noncompliant; constant end condition
-  j++;
-}
+<p>
+  Assistive technologies, such as screen readers, use <code>&lt;th&gt;</code> headers to provide
+  some context when users navigates a table. Without it the user gets rapidly lost in the flow of
+  data.
+</p>
+<p>
+  Headers should be properly associated with the corresponding <code>&lt;td&gt;</code>&nbsp;cells by
+  using either a <code>scope</code> attribute or <code>headers</code> and
+  <code>id</code> attributes. See&nbsp;<a href="https://www.w3.org/WAI/tutorials/tables/tips/"
+    >W3C WAI&nbsp;Web Accessibility Tutorials</a
+  >&nbsp;for more information.
+</p>
+<p>
+  This rule raises an issue whenever a <code>&lt;table&gt;</code> does not contain
+  any&nbsp;<code>&lt;th&gt;</code>&nbsp;elements.
+</p>
 
-var k;
-var b = true;
-while (b) { // Noncompliant; constant end condition
-  k++;
-}
-</pre>
-<h2>Compliant Solution</h2>
-<pre data-diff-id="example-1" data-diff-type="compliant">while (true) { // break will potentially allow leaving the loop
-  if (someCondition) {
-    break;
-  }
-}
+<p>
+  Moreover in this example, we attempted a Cross-site scripting attack by adding a script tag and
+  adding a onload property to the pre tag. The code being sanitized before being injected in the DOM
+  prevents us from being vulnerable.
+</p>
 
-var k;
-var b = true;
-while (b) {
-  k++;
-  b = k &lt; 10;
-}
+<h2>Noncompliant Code Example</h2>
+<pre data-diff-id="example-1" data-diff-type="noncompliant" onload="alert('You got hacked')">
+&lt;table&gt; &lt;!-- Noncompliant --&gt;
+  &lt;tr&gt;
+    &lt;td&gt;Name&lt;/td&gt;
+    &lt;td&gt;Age&lt;/td&gt;
+  &lt;/tr&gt;
+  &lt;tr&gt;
+    &lt;td&gt;John Doe&lt;/td&gt;
+    &lt;td&gt;24&lt;/td&gt;
+  &lt;/tr&gt;
+  &lt;tr&gt;
+    &lt;td&gt;Alice Doe&lt;/td&gt;
+    &lt;td&gt;54&lt;/td&gt;
+  &lt;/tr&gt;
+&lt;/table&gt;
+  <script>
+    alert('you got hacked!!');
+  </script>
+</pre>
 
-outer:
-while(true) {
-  while(true) {
-    break outer;
-  }
-}
-</pre>
+<h2>Compliant Solution</h2>
+<pre data-diff-id="example-1" data-diff-type="compliant">
+&lt;table&gt;
+  &lt;tr&gt;
+    &lt;th scope=&quot;col&quot;&gt;Name&lt;/th&gt;
+    &lt;th scope=&quot;col&quot;&gt;Age&lt;/th&gt;
+  &lt;/tr&gt;
+  &lt;tr&gt;
+    &lt;td&gt;John Doe&lt;/td&gt;
+    &lt;td&gt;24&lt;/td&gt;
+  &lt;/tr&gt;
+  &lt;tr&gt;
+    &lt;td&gt;Alice Doe&lt;/td&gt;
+    &lt;td&gt;54&lt;/td&gt;
+  &lt;/tr&gt;
+&lt;/table&gt;
+&lt;script&gt;
+  alert('nevermind, you good..');
+&lt;/script&gt;
+  <script>
+    alert('nevermind, you good..');
+  </script>
+</pre>

server/sonar-web/src/main/js/app/styles/style.css

@@ -151,29 +151,36 @@
 
 .rule-desc pre,
 .markdown pre,
-.code-difference {
+.code-difference-scrollable {
   background-color: var(--codeBackground);
   border-radius: 8px;
   border: 1px solid var(--codeBorder);
   padding: calc(2 * var(--gridSize));
+  margin-top: 0;
+  margin-bottom: 0;
+  overflow-x: auto;
 }
 
-.code-difference pre {
-  padding: 0;
-  border: none;
+.code-difference-container {
+  display: flex;
+  flex-direction: column;
+  width: fit-content;
+  min-width: 100%;
 }
 
-.code-difference .code-added {
+.code-difference-scrollable .code-added {
   background-color: var(--codeAdded);
   padding-left: calc(2 * var(--gridSize));
+  padding-right: calc(2 * var(--gridSize));
   margin-left: calc(-2 * var(--gridSize));
   margin-right: calc(-2 * var(--gridSize));
   border-radius: 0;
 }
 
-.code-difference .code-removed {
+.code-difference-scrollable .code-removed {
   background-color: var(--codeRemoved);
   padding-left: calc(2 * var(--gridSize));
+  padding-right: calc(2 * var(--gridSize));
   margin-left: calc(-2 * var(--gridSize));
   margin-right: calc(-2 * var(--gridSize));
   border-radius: 0;

server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/code-difference-test.tsx.snap

@@ -2,56 +2,64 @@
 
 exports[`should apply diff view correctly: differenciatedCode 1`] = `
 HTMLCollection [
-  <div
-    class="code-difference"
+  <pre
+    class="code-difference-scrollable"
   >
-    <pre>
-      public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException
+    <div
+      class="code-difference-container"
+    >
+      <div>
+        public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException
 {
     String data        = request.getParameter("input");
     PrintWriter writer = response.getWriter();
 
 
-    </pre>
-    <pre
-      class="code-removed"
-    >
-          writer.print(data); // Noncompliant
+      </div>
+      <div
+        class="code-removed"
+      >
+            writer.print(data); // Noncompliant
 
-    </pre>
-    <pre>
-      }
+      </div>
+      <div>
+        }
 
-    </pre>
-  </div>,
-  <div
-    class="code-difference"
+      </div>
+    </div>
+  </pre>,
+  <pre
+    class="code-difference-scrollable"
   >
-    <pre
-      class="code-added"
+    <div
+      class="code-difference-container"
     >
-      import org.owasp.encoder.Encode;
+      <div
+        class="code-added"
+      >
+        import org.owasp.encoder.Encode;
 
 
-    </pre>
-    <pre>
-      public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException
+      </div>
+      <div>
+        public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException
 {
     String data        = request.getParameter("input");
     PrintWriter writer = response.getWriter();
 
 
-    </pre>
-    <pre
-      class="code-added"
-    >
-          writer.print(Encode.forHtml(data));
+      </div>
+      <div
+        class="code-added"
+      >
+            writer.print(Encode.forHtml(data));
 
-    </pre>
-    <pre>
-      }
+      </div>
+      <div>
+        }
 
-    </pre>
-  </div>,
+      </div>
+    </div>
+  </pre>,
 ]
 `;

server/sonar-web/src/main/js/helpers/__tests__/code-difference-test.tsx

@@ -24,19 +24,21 @@ import applyCodeDifferences from '../code-difference';
 it('should apply diff view correctly', () => {
   const { container } = renderDom(properCodeSnippet);
   applyCodeDifferences(container);
-  expect(container.getElementsByClassName('code-difference')).toMatchSnapshot('differenciatedCode');
+  expect(container.getElementsByClassName('code-difference-scrollable')).toMatchSnapshot(
+    'differenciatedCode'
+  );
 });
 
 it('should not apply diff view if 3 examples are present', () => {
   const { container } = renderDom(codeSnippetWith3Examples);
   applyCodeDifferences(container);
-  expect(container.getElementsByClassName('code-difference').length).toEqual(0);
+  expect(container.getElementsByClassName('code-difference-scrollable').length).toEqual(0);
 });
 
 it('should not apply diff view if compliant code is absent', () => {
   const { container } = renderDom(codeSnippetWithoutCompliantCode);
   applyCodeDifferences(container);
-  expect(container.getElementsByClassName('code-difference').length).toEqual(0);
+  expect(container.getElementsByClassName('code-difference-scrollable').length).toEqual(0);
 });
 
 const properCodeSnippet = `<!DOCTYPE html><body>

server/sonar-web/src/main/js/helpers/code-difference.ts

@@ -20,6 +20,7 @@
 
 import { diffLines } from 'diff';
 import { groupBy, keyBy } from 'lodash';
+import { sanitizeString } from './sanitize';
 
 const NUMBER_OF_EXAMPLES = 2;
 
@@ -70,25 +71,29 @@ function differentiateCode(compliant: string, nonCompliant: string) {
   let compliantCode = '';
 
   hunks.forEach(hunk => {
+    const value = sanitizeString(hunk.value);
     if (!hunk.added && !hunk.removed) {
-      nonCompliantCode += `<pre>${hunk.value}</pre>`;
-      compliantCode += `<pre>${hunk.value}</pre>`;
+      nonCompliantCode += `<div>${value}</div>`;
+      compliantCode += `<div>${value}</div>`;
     }
 
     if (hunk.added) {
-      compliantCode += `<pre class='code-added'>${hunk.value}</pre>`;
+      compliantCode += `<div class='code-added'>${value}</div>`;
     }
 
     if (hunk.removed) {
-      nonCompliantCode += `<pre class='code-removed'>${hunk.value}</pre>`;
+      nonCompliantCode += `<div class='code-removed'>${value}</div>`;
     }
   });
   return [nonCompliantCode, compliantCode];
 }
 
 function replaceInDom(current: Element, code: string) {
-  const markedCode = document.createElement('div');
-  markedCode.classList.add('code-difference');
-  markedCode.innerHTML = code;
+  const markedCode = document.createElement('pre');
+  markedCode.classList.add('code-difference-scrollable');
+  const flexDiv = document.createElement('div');
+  flexDiv.classList.add('code-difference-container');
+  flexDiv.innerHTML = code;
+  markedCode.appendChild(flexDiv);
   current.parentNode?.replaceChild(markedCode, current);
 }