Skip to content

Commit

Permalink
Asserts sanitization is strong enough
Browse files Browse the repository at this point in the history
  • Loading branch information
PowerKiKi committed Sep 6, 2024
1 parent cb4234e commit ef8fc05
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 18 deletions.
19 changes: 2 additions & 17 deletions src/js/Item.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {ModelAttributes} from './galleries/AbstractGallery';
import {sanitizeHtml} from "./Utility";

export declare interface ItemOptions {
lightbox?: boolean;
Expand All @@ -8,12 +9,6 @@ export declare interface ItemOptions {
showLabels?: 'hover' | 'never' | 'always';
}

export interface ItemTitle {
title: string;
link: string;
linkTarget: '_blank' | '_self' | '_parent' | '_top';
}

export type ItemActivateEventDetail<Model extends ModelAttributes> = {
clickEvent: MouseEvent;
item: Item<Model>;
Expand Down Expand Up @@ -72,24 +67,14 @@ export class Item<Model extends ModelAttributes> {

/**
*
* @param {ItemOptions} options
* @param model Contains the source data given for an item (e.g object instance from database with id etc..)
*/
public constructor(
private readonly document: Document,
private readonly options: ItemOptions,
public readonly model: Model,
) {
this.title = this.getTitleDetails(model.title);
}

/**
* Cleans html, and returns only the text from all eventual tags
* @param {string} term
* @returns {ItemTitle}
*/
private getTitleDetails(term: string | undefined): string {
return term ? term.replace(/<(?!\s*br\s*\/?)[^>]+>/gi, '') : '';
this.title = sanitizeHtml(model.title);
}

/**
Expand Down
7 changes: 7 additions & 0 deletions src/js/Utility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,10 @@ export function getImageRatioAndIfCropped(model: SizedModel, ratioLimits?: Ratio

return {ratio: ratio, cropped: cropped};
}

/**
* Cleans HTML, and returns only the plain text and `<br>` from all eventual tags
*/
export function sanitizeHtml(term: string | undefined): string {
return term ? term.replace(/<(?!\s*br\s*\/?)[^>]*>/gi, '') : '';
}
39 changes: 38 additions & 1 deletion testing/unit/utility.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {getIcon, getImageRatio} from '../../src/js/Utility';
import {getIcon, getImageRatio, sanitizeHtml} from '../../src/js/Utility';
import * as domino from 'domino';

describe('Utility', () => {
Expand Down Expand Up @@ -26,3 +26,40 @@ describe('Utility', () => {
expect(svg.outerHTML).toBe('<svg viewBox="0 0 100 100"><use xlink:href="#my-name"></use></svg>');
});
});

describe('sanitizeHtml', () => {

test('should not touch plain text and <br>', () => {
expect(sanitizeHtml('')).toBe('');
expect(sanitizeHtml(' ')).toBe(' ');
expect(sanitizeHtml('foo')).toBe('foo');
expect(sanitizeHtml('one > two > three')).toBe('one > two > three');
expect(sanitizeHtml('foo <br/>bar')).toBe('foo <br/>bar');
expect(sanitizeHtml('foo <br />bar')).toBe('foo <br />bar');
expect(sanitizeHtml('foo <br>bar')).toBe('foo <br>bar');
expect(sanitizeHtml('foo <BR>bar')).toBe('foo <BR>bar');
expect(sanitizeHtml('foo <br >bar')).toBe('foo <br >bar');
});

test('should remove most HTML tag', () => {
expect(sanitizeHtml('<strong>')).toBe('');
expect(sanitizeHtml('<strong></strong>')).toBe('');
expect(sanitizeHtml('foo <strong>bar</strong> baz')).toBe('foo bar baz');
expect(sanitizeHtml('foo <unknown-tag>bar</unknown-tag> baz')).toBe('foo bar baz');
expect(sanitizeHtml('foo <unknown-tag attr="val">bar</unknown-tag> baz')).toBe('foo bar baz');
expect(sanitizeHtml('foo <unknown-tag \n attr="val">bar</unknown-tag> baz')).toBe('foo bar baz');
expect(sanitizeHtml('foo \n<unknown-tag \nattr="val">bar\n</unknown-tag> \nbaz')).toBe('foo \nbar\n \nbaz');
expect(sanitizeHtml('<STRONG>foo<STRONG> <strong>bar</strong> <em>baz</em>')).toBe('foo bar baz');
expect(sanitizeHtml('<strong>one</strong> > two > three')).toBe('one > two > three');
expect(sanitizeHtml('<scrip<script>is removed</script>t>alert(123)</script>')).toBe('is removedt>alert(123)'); // Broken but safe HTML
expect(sanitizeHtml('<!<!--- comment --->>')).toBe('>'); // Broken but safe HTML
expect(sanitizeHtml('a<>b')).toBe('ab');
expect(sanitizeHtml('a</>b')).toBe('ab');
expect(sanitizeHtml('a<>b</>c')).toBe('abc');
expect(
sanitizeHtml(`<div
>foo</
div>`),
).toBe('foo');
});
});

0 comments on commit ef8fc05

Please # to comment.