Skip to content

Commit 8fb4772

Browse files
committed
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
1 parent 779ff47 commit 8fb4772

File tree

2 files changed

+54
-6
lines changed

2 files changed

+54
-6
lines changed

src/app/shared/utils/markdown.directive.spec.ts

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,20 @@ import {
88
} from '@angular/core/testing';
99
import { By } from '@angular/platform-browser';
1010

11+
import { environment } from '../../../environments/environment.test';
1112
import { MathService } from '../../core/shared/math.service';
1213
import { MockMathService } from '../../core/shared/math.service.spec';
1314
import { MarkdownDirective } from './markdown.directive';
1415

1516
@Component({
16-
template: `<div dsMarkdown="test"></div>`,
17+
template: `<div [dsMarkdown]="'test<script>alert(1);</script>'"></div>`,
1718
standalone: true,
1819
imports: [ MarkdownDirective ],
1920
})
2021
class TestComponent {}
2122

2223
describe('MarkdownDirective', () => {
23-
let component: TestComponent;
2424
let fixture: ComponentFixture<TestComponent>;
25-
let divEl: DebugElement;
2625

2726
beforeEach(async () => {
2827
await TestBed.configureTestingModule({
@@ -32,12 +31,61 @@ describe('MarkdownDirective', () => {
3231
}).compileComponents();
3332
spyOn(MarkdownDirective.prototype, 'render');
3433
fixture = TestBed.createComponent(TestComponent);
35-
component = fixture.componentInstance;
36-
divEl = fixture.debugElement.query(By.css('div'));
3734
});
3835

3936
it('should call render method', () => {
4037
fixture.detectChanges();
4138
expect(MarkdownDirective.prototype.render).toHaveBeenCalled();
4239
});
40+
41+
});
42+
43+
describe('MarkdownDirective sanitization with markdown disabled', () => {
44+
let fixture: ComponentFixture<TestComponent>;
45+
let divEl: DebugElement;
46+
// Disable markdown
47+
environment.markdown.enabled = false;
48+
49+
beforeEach(async () => {
50+
await TestBed.configureTestingModule({
51+
providers: [
52+
{ provide: MathService, useClass: MockMathService },
53+
],
54+
}).compileComponents();
55+
fixture = TestBed.createComponent(TestComponent);
56+
divEl = fixture.debugElement.query(By.css('div'));
57+
58+
});
59+
60+
it('should sanitize the script element out of innerHTML (markdown disabled)',() => {
61+
fixture.detectChanges();
62+
divEl = fixture.debugElement.query(By.css('div'));
63+
expect(divEl.nativeElement.innerHTML).toEqual('test');
64+
});
65+
66+
});
67+
68+
describe('MarkdownDirective sanitization with markdown enabled', () => {
69+
let fixture: ComponentFixture<TestComponent>;
70+
let divEl: DebugElement;
71+
// Enable markdown
72+
environment.markdown.enabled = true;
73+
74+
beforeEach(async () => {
75+
await TestBed.configureTestingModule({
76+
providers: [
77+
{ provide: MathService, useClass: MockMathService },
78+
],
79+
}).compileComponents();
80+
fixture = TestBed.createComponent(TestComponent);
81+
divEl = fixture.debugElement.query(By.css('div'));
82+
83+
});
84+
85+
it('should sanitize the script element out of innerHTML (markdown enabled)',() => {
86+
fixture.detectChanges();
87+
divEl = fixture.debugElement.query(By.css('div'));
88+
expect(divEl.nativeElement.innerHTML).toEqual('test');
89+
});
90+
4391
});

src/app/shared/utils/markdown.directive.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class MarkdownDirective implements OnInit, OnDestroy {
5555

5656
async render(value: string, forcePreview = false): Promise<SafeHtml> {
5757
if (isEmpty(value) || (!environment.markdown.enabled && !forcePreview)) {
58-
this.el.innerHTML = value;
58+
this.el.innerHTML = this.sanitizer.sanitize(SecurityContext.HTML, value);
5959
return;
6060
} else {
6161
if (environment.markdown.mathjax) {

0 commit comments

Comments
 (0)