Merge pull request #3375 from kshepherd/markdown_directive_force_sani…

…tization

Always sanitize HTML passed to dsMarkdown even if markdown rendering is disabled

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');
+  });
+
 });

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) {