Skip to content

Commit

Permalink
Always sanitize HTML in dsMarkdown even if markdown disabled
Browse files Browse the repository at this point in the history
Instead of setting innerHTML directly to value, sanitize
the value, even if not passing to renderMarkdown/Mathjax
  • Loading branch information
kshepherd committed Oct 1, 2024
1 parent 779ff47 commit 8fb4772
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
58 changes: 53 additions & 5 deletions src/app/shared/utils/markdown.directive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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');
});

});
2 changes: 1 addition & 1 deletion src/app/shared/utils/markdown.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 8fb4772

Please sign in to comment.