From 8fb4772b6c6308d2ba9e358e2858244105064c7f Mon Sep 17 00:00:00 2001
From: Kim Shepherd <kim@shepherd.nz>
Date: Tue, 1 Oct 2024 13:12:10 +0200
Subject: [PATCH] Always sanitize HTML in dsMarkdown even if markdown disabled

Instead of setting innerHTML directly to value, sanitize
the value, even if not passing to renderMarkdown/Mathjax
---
 .../shared/utils/markdown.directive.spec.ts   | 58 +++++++++++++++++--
 src/app/shared/utils/markdown.directive.ts    |  2 +-
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/src/app/shared/utils/markdown.directive.spec.ts b/src/app/shared/utils/markdown.directive.spec.ts
index 79e795be876..b984543c40d 100644
--- a/src/app/shared/utils/markdown.directive.spec.ts
+++ b/src/app/shared/utils/markdown.directive.spec.ts
@@ -8,21 +8,20 @@ import {
 } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
 
+import { environment } from '../../../environments/environment.test';
 import { MathService } from '../../core/shared/math.service';
 import { MockMathService } from '../../core/shared/math.service.spec';
 import { MarkdownDirective } from './markdown.directive';
 
 @Component({
-  template: `<div dsMarkdown="test"></div>`,
+  template: `<div [dsMarkdown]="'test<script>alert(1);</script>'"></div>`,
   standalone: true,
   imports: [ MarkdownDirective ],
 })
 class TestComponent {}
 
 describe('MarkdownDirective', () => {
-  let component: TestComponent;
   let fixture: ComponentFixture<TestComponent>;
-  let divEl: DebugElement;
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({
@@ -32,12 +31,61 @@ describe('MarkdownDirective', () => {
     }).compileComponents();
     spyOn(MarkdownDirective.prototype, 'render');
     fixture = TestBed.createComponent(TestComponent);
-    component = fixture.componentInstance;
-    divEl = fixture.debugElement.query(By.css('div'));
   });
 
   it('should call render method', () => {
     fixture.detectChanges();
     expect(MarkdownDirective.prototype.render).toHaveBeenCalled();
   });
+
+});
+
+describe('MarkdownDirective sanitization with markdown disabled', () => {
+  let fixture: ComponentFixture<TestComponent>;
+  let divEl: DebugElement;
+  // Disable markdown
+  environment.markdown.enabled = false;
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      providers: [
+        { provide: MathService, useClass: MockMathService },
+      ],
+    }).compileComponents();
+    fixture = TestBed.createComponent(TestComponent);
+    divEl = fixture.debugElement.query(By.css('div'));
+
+  });
+
+  it('should sanitize the script element out of innerHTML (markdown disabled)',() => {
+    fixture.detectChanges();
+    divEl = fixture.debugElement.query(By.css('div'));
+    expect(divEl.nativeElement.innerHTML).toEqual('test');
+  });
+
+});
+
+describe('MarkdownDirective sanitization with markdown enabled', () => {
+  let fixture: ComponentFixture<TestComponent>;
+  let divEl: DebugElement;
+  // Enable markdown
+  environment.markdown.enabled = true;
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      providers: [
+        { provide: MathService, useClass: MockMathService },
+      ],
+    }).compileComponents();
+    fixture = TestBed.createComponent(TestComponent);
+    divEl = fixture.debugElement.query(By.css('div'));
+
+  });
+
+  it('should sanitize the script element out of innerHTML (markdown enabled)',() => {
+    fixture.detectChanges();
+    divEl = fixture.debugElement.query(By.css('div'));
+    expect(divEl.nativeElement.innerHTML).toEqual('test');
+  });
+
 });
diff --git a/src/app/shared/utils/markdown.directive.ts b/src/app/shared/utils/markdown.directive.ts
index 0540e25cc54..de13acb3036 100644
--- a/src/app/shared/utils/markdown.directive.ts
+++ b/src/app/shared/utils/markdown.directive.ts
@@ -55,7 +55,7 @@ export class MarkdownDirective implements OnInit, OnDestroy {
 
   async render(value: string, forcePreview = false): Promise<SafeHtml> {
     if (isEmpty(value) || (!environment.markdown.enabled && !forcePreview)) {
-      this.el.innerHTML = value;
+      this.el.innerHTML = this.sanitizer.sanitize(SecurityContext.HTML, value);
       return;
     } else {
       if (environment.markdown.mathjax) {