b3cffba5-e159-4c1b-9c4b-13e5d8490b0e

Build # b3cffba5-e159-4c1b-9c4b-13e5d8490b0e

Build Type

circleci

Committed by Sebastian Rettig

Commit Message

feat(xss): all entrypoints of H5PEditor validate parameters

Pull Request Pull Request #882: feat(security): text in content is filtered on the server

Run Details

1163 of 1591 branches covered (73.1%)

Branch coverage included in aggregate %.

78 of 90 new or added lines in 4 files covered. (86.67%)

1 existing line in 1 file now uncovered.

2918 of 3253 relevant lines covered (89.7%)

6924.72 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

86.71

/src/SemanticsEnforcer.ts

import sanitizeHtml from 'sanitize-html';
import { escape } from 'html-escaper';

import { ContentScanner } from './ContentScanner';
import LibraryManager from './LibraryManager';
import Logger from './helpers/Logger';
import { ILibraryName, ISemanticsEntry } from './types';

const log = new Logger('SemanticsEnforcer');

/**
 * Validates the params received by the editor or when uploading content against
 * the schema specified in the semantic structure of the H5P libraries used for
 * the content.
 *
 * IMPORTANT: This class is very incomplete and mostly only a stub!
 */
export default class SemanticsEnforcer {
    constructor(libraryManager: LibraryManager) {
        log.info('initialize');
        this.contentScanner = new ContentScanner(libraryManager);
    }

    private contentScanner: ContentScanner;

    /**
     * Makes sure the params follow the semantic structure of the library.
     * IMPORTANT: This method changes the contents of mainParams!
     * @param mainParams the params; THIS PARAMETER IS MANIPULATED BY THIS
     * METHOD!
     * @param mainLibraryName
     */
    public async enforceSemanticStructure(
        mainParams: any,
        mainLibraryName: ILibraryName
    ): Promise<void> {
        log.debug('Starting to enforcing semantic structure.');
        await this.contentScanner.scanContent(
            mainParams,
            mainLibraryName,
            (semantics, params, jsonPath, parentParams) => {
                let deleteMe = false;
                if (semantics.type === 'library') {
                    const result = this.enforceLibrarySemantics(
                        semantics,
                        params,
                        parentParams,
                        jsonPath
                    );
                    deleteMe = result.deleteMe;
                } else if (semantics.type === 'text') {
                    this.enforceTextSemantics(
                        semantics,
                        params,
                        parentParams,
                        jsonPath
                    );
                }

                if (deleteMe) {
                    log.debug(
                        `Enforcing library semantics by deleting params path ${jsonPath}.`
                    );
                    delete parentParams[semantics.name];
                    return true;
                }
                return false;
            }
        );
    }

    /**
     * See h5p.classes.php:3994 for how the PHP implementation does this.
     * @param semantics
     * @param params
     * @param parentParams
     * @param jsonPath
     */
    private enforceLibrarySemantics(
        semantics: ISemanticsEntry,
        params: any,
        parentParams: any,
        jsonPath: string
    ): { deleteMe: boolean } {
        log.debug('Enforcing structure of a library element.');
        let deleteMe: boolean;
        if (!params.library) {
            log.debug(
                `Found a library entry without a library property (${jsonPath}). Marking it for deletion.`
            );
            // We silently delete the params in this case, as the H5P editor
            // sometimes creates malformed library entries and this is not an
            // error by the user.
            deleteMe = true;
        } else {
            const allowedLibraries = semantics.options
                ?.map((o) => (typeof o === 'string' ? o : o.name))
                ?.filter((o) => o);
            if (
                !allowedLibraries ||
                !allowedLibraries.includes(params.library)
            ) {
                log.debug(
                    `Library check: Found a library entry with a library that is not specified in the options. Allowed are: ${allowedLibraries.join(
                        ', '
                    )}. Library is set to: ${params.library} (${jsonPath}).`
                );
                deleteMe = true;
            }
        }
        // Further checks that should be performed here:
        // - Check if library version is only a different version from the one
        //   specified in options, or if library is not allowed in general.
        //   (delete the params entry in either case, but report different
        //   errors)
        // - Validate subcontent metadata (.metadata property)
        // - Filter out params that are not valid per se (only library, params,
        //   subContentId, metadata and the list in 'extraAttributes' is
        //   allowed). Remove all other keys in params.
        // - If subContentId exists: check if it is a properly formatted string
        //   (/^\{?[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}\}?$/)
        //   and delete the subContentId if it's not.

        return {
            deleteMe
        };
    }

    /**
     * Checks if a text entry conforms to the semantics and enforces the
     * semantics if necessary. Implements all checks of the PHP version, but
     * does not include error reporting at the moment.
     */
    private enforceTextSemantics(
        semantics: ISemanticsEntry,
        params: any,
        parentParams: any,
        jsonPath: string
    ): void {
        // TODO:
        // - respect detailed font specifications in style filter
        // - report errors to user

        log.debug('Checking contents of text entry');
        let newText: string = params;
        if (!newText) {
            newText = '';
        }

        // Filter out disallowed HTML tags to prevent XSS attacks.
        if (semantics.tags) {
            // We always allow divs, pans, p and br.
            let allowedTags = ['div', 'span', 'p', 'br', ...semantics.tags];
            // We add related HTML tags
            if (allowedTags.includes('table')) {
                allowedTags = [
                    ...allowedTags,
                    'tr',
                    'td',
                    'th',
                    'colgroup',
                    'thead',
                    'tbody',
                    'tfoot'
                ];
            }
            if (allowedTags.includes('strong')) {
                allowedTags.push('b');
            } else {
                if (allowedTags.includes('b')) {
                    allowedTags.push('strong');
                }
            }
            if (allowedTags.includes('em')) {
                allowedTags.push('i');
            } else {
                if (allowedTags.includes('i')) {
                    allowedTags.push('em');
                }
            }
            if (allowedTags.includes('ul') || allowedTags.includes('ol')) {
                allowedTags.push('li');
            }
            if (allowedTags.includes('del') || allowedTags.includes('strike')) {
                allowedTags.push('s');
            }
            const uniqueTags = new Set<string>(allowedTags);
            // We never allow inherently unsafe tags, even if the library would
            // allow this.
            uniqueTags.delete('script');
            uniqueTags.delete('style');
            uniqueTags.delete('textarea');
            uniqueTags.delete('option');
            allowedTags = Array.from(uniqueTags);

            // Text alignment is always allowed.
            const allowedStyles = {
                'text-align': [/(center|left|right)/i]
            };
            // We only allow the styles set in the semantics.
            if (semantics.font) {
                if (semantics.font.size) {
                    allowedStyles['font-size'] = [/^[0-9.]+(em|px|%)$/i];
                }
                if (semantics.font.family) {
                    allowedStyles['font-family'] = [/^[-a-z0-9," ]+$/i];
                }
                if (semantics.font.color) {
                    // tslint:disable-next-line: no-string-literal
                    allowedStyles['color'] = [
                        /^(#[a-f0-9]{3}[a-f0-9]{3}?|rgba?\([0-9, ]+\))$/i
                    ];
                }
                if (semantics.font.background) {
                    allowedStyles['background-color'] = [
                        /^(#[a-f0-9]{3}[a-f0-9]{3}?|rgba?\([0-9, ]+\))$/i
                    ];
                }
                if (semantics.font.spacing) {
                    // tslint:disable-next-line: no-string-literal
                    allowedStyles['spacing'] = [/^[0-9.]+(em|px|%)$/i];
                }
                if (semantics.font.height) {
                    allowedStyles['line-height'] = [/^[0-9.]+(em|px|%)$/i];
                }
            }

            log.debug('Filtering out disallowed HTML tags');

            newText = sanitizeHtml(newText, {
                allowedTags,
                allowedAttributes: {
                    '*': ['style'], // styles are filtered, so we can allow this
                    a: ['href', 'hreflang', 'media', 'rel', 'target'],
                    td: ['colspan', 'rowspan', 'headers'],
                    th: ['colspan', 'rowspan', 'headers', 'scope'],
                    ol: ['start', 'reversed'],
                    table: ['summary']
                },
                allowedStyles: { '*': allowedStyles }
            });
        } else {
            log.debug('Filtering out all HTML tags');
            // Escape all HTML tags if the field doesn't allow HTML in general.
            newText = escape(newText);
        }

        // Check if string has the required length
        if (semantics.maxLength && newText.length > semantics.maxLength) {
            log.debug('Clipping string that is too long');
            newText = newText.substring(0, semantics.maxLength);
        }

        // Check if string conforms to regexp (if given and if set to optional)
        if (newText !== '' && semantics.optional && semantics.regexp) {
            const regexp = new RegExp(
                semantics.regexp.pattern,
                semantics.regexp.modifiers
            );
            if (!regexp.test(newText)) {
                log.debug(
                    `Provided string does not conform to regexp in semantics (value: ${newText}, regexp: ${semantics.regexp})`
                );
                newText = '';
            }
        }

        parentParams[semantics.name] = newText;
    }
}

1	import sanitizeHtml from 'sanitize-html';	25✔
2	import { escape } from 'html-escaper';	25✔
3
4	import { ContentScanner } from './ContentScanner';	25✔
5	import LibraryManager from './LibraryManager';
6	import Logger from './helpers/Logger';	25✔
7	import { ILibraryName, ISemanticsEntry } from './types';
8
9	const log = new Logger('SemanticsEnforcer');	25✔
10
11	/**
12	* Validates the params received by the editor or when uploading content against
13	* the schema specified in the semantic structure of the H5P libraries used for
14	* the content.
15	*
16	* IMPORTANT: This class is very incomplete and mostly only a stub!
17	*/
18	export default class SemanticsEnforcer {	25✔
19	constructor(libraryManager: LibraryManager) {
20	log.info('initialize');	128✔
21	this.contentScanner = new ContentScanner(libraryManager);	128✔
22	}
23
24	private contentScanner: ContentScanner;
25
26	/**
27	* Makes sure the params follow the semantic structure of the library.
28	* IMPORTANT: This method changes the contents of mainParams!
29	* @param mainParams the params; THIS PARAMETER IS MANIPULATED BY THIS
30	* METHOD!
31	* @param mainLibraryName
32	*/
33	public async enforceSemanticStructure(	25✔
34	mainParams: any,
35	mainLibraryName: ILibraryName
36	): Promise<void> {
37	log.debug('Starting to enforcing semantic structure.');	81✔
38	await this.contentScanner.scanContent(	162✔
39	mainParams,
40	mainLibraryName,
41	(semantics, params, jsonPath, parentParams) => {
42	let deleteMe = false;	13,854✔
43	if (semantics.type === 'library') {	13,854✔
44	const result = this.enforceLibrarySemantics(	494✔
45	semantics,
46	params,
47	parentParams,
48	jsonPath
49	);
50	deleteMe = result.deleteMe;	494✔
51	} else if (semantics.type === 'text') {	13,360✔
52	this.enforceTextSemantics(	5,553✔
53	semantics,
54	params,
55	parentParams,
56	jsonPath
57	);
58	}
59
60	if (deleteMe) {	13,854!
61	log.debug(	×
62	`Enforcing library semantics by deleting params path ${jsonPath}.`
63	);
64	delete parentParams[semantics.name];	×
65	return true;	×
66	}
67	return false;	13,854✔
68	}
69	);
70	}
71
72	/**
73	* See h5p.classes.php:3994 for how the PHP implementation does this.
74	* @param semantics
75	* @param params
76	* @param parentParams
77	* @param jsonPath
78	*/
79	private enforceLibrarySemantics(	25✔
80	semantics: ISemanticsEntry,
81	params: any,
82	parentParams: any,
83	jsonPath: string
84	): { deleteMe: boolean } {
85	log.debug('Enforcing structure of a library element.');	494✔
86	let deleteMe: boolean;
87	if (!params.library) {	494!
88	log.debug(	×
89	`Found a library entry without a library property (${jsonPath}). Marking it for deletion.`
90	);
91	// We silently delete the params in this case, as the H5P editor
92	// sometimes creates malformed library entries and this is not an
93	// error by the user.
94	deleteMe = true;	×
95	} else {
96	const allowedLibraries = semantics.options	494!
97	?.map((o) => (typeof o === 'string' ? o : o.name))	5,982!
98	?.filter((o) => o);	5,982✔
99	if (	494!
100	!allowedLibraries \|\|	988✔
101	!allowedLibraries.includes(params.library)
102	) {
103	log.debug(	×
104	`Library check: Found a library entry with a library that is not specified in the options. Allowed are: ${allowedLibraries.join(
105	', '
106	)}. Library is set to: ${params.library} (${jsonPath}).`
107	);
108	deleteMe = true;	×
109	}
110	}
111	// Further checks that should be performed here:
112	// - Check if library version is only a different version from the one
113	// specified in options, or if library is not allowed in general.
114	// (delete the params entry in either case, but report different
115	// errors)
116	// - Validate subcontent metadata (.metadata property)
117	// - Filter out params that are not valid per se (only library, params,
118	// subContentId, metadata and the list in 'extraAttributes' is
119	// allowed). Remove all other keys in params.
120	// - If subContentId exists: check if it is a properly formatted string
121	// (/^\{?[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}\}?$/)
122	// and delete the subContentId if it's not.
123
124	return {	494✔
125	deleteMe
126	};
127	}
128
129	/**
130	* Checks if a text entry conforms to the semantics and enforces the
131	* semantics if necessary. Implements all checks of the PHP version, but
132	* does not include error reporting at the moment.
133	*/
134	private enforceTextSemantics(	25✔
135	semantics: ISemanticsEntry,
136	params: any,
137	parentParams: any,
138	jsonPath: string
139	): void {
140	// TODO:
141	// - respect detailed font specifications in style filter
142	// - report errors to user
143
144	log.debug('Checking contents of text entry');	5,553✔
145	let newText: string = params;	5,553✔
146	if (!newText) {	5,553✔
147	newText = '';	612✔
148	}
149
150	// Filter out disallowed HTML tags to prevent XSS attacks.
151	if (semantics.tags) {	5,553✔
152	// We always allow divs, pans, p and br.
153	let allowedTags = ['div', 'span', 'p', 'br', ...semantics.tags];	1,135✔
154	// We add related HTML tags
155	if (allowedTags.includes('table')) {	1,135✔
156	allowedTags = [	12✔
157	...allowedTags,
158	'tr',
159	'td',
160	'th',
161	'colgroup',
162	'thead',
163	'tbody',
164	'tfoot'
165	];
166	}
167	if (allowedTags.includes('strong')) {	1,135✔
168	allowedTags.push('b');	998✔
169	} else {
170	if (allowedTags.includes('b')) {	137✔
171	allowedTags.push('strong');	3✔
172	}
173	}
174	if (allowedTags.includes('em')) {	1,135✔
175	allowedTags.push('i');	997✔
176	} else {
177	if (allowedTags.includes('i')) {	138!
NEW 178	allowedTags.push('em');	×
179	}
180	}
181	if (allowedTags.includes('ul') \|\| allowedTags.includes('ol')) {	1,135✔
182	allowedTags.push('li');	211✔
183	}
184	if (allowedTags.includes('del') \|\| allowedTags.includes('strike')) {	1,135✔
185	allowedTags.push('s');	470✔
186	}
187	const uniqueTags = new Set<string>(allowedTags);	1,135✔
188	// We never allow inherently unsafe tags, even if the library would
189	// allow this.
190	uniqueTags.delete('script');	1,135✔
191	uniqueTags.delete('style');	1,135✔
192	uniqueTags.delete('textarea');	1,135✔
193	uniqueTags.delete('option');	1,135✔
194	allowedTags = Array.from(uniqueTags);	1,135✔
195
196	// Text alignment is always allowed.
197	const allowedStyles = {	1,135✔
198	'text-align': [/(center\|left\|right)/i]
199	};
200	// We only allow the styles set in the semantics.
201	if (semantics.font) {	1,135✔
202	if (semantics.font.size) {	43✔
203	allowedStyles['font-size'] = [/^[0-9.]+(em\|px\|%)$/i];	31✔
204	}
205	if (semantics.font.family) {	43!
NEW 206	allowedStyles['font-family'] = [/^[-a-z0-9," ]+$/i];	×
207	}
208	if (semantics.font.color) {	43✔
209	// tslint:disable-next-line: no-string-literal
210	allowedStyles['color'] = [	41✔
211	/^(#[a-f0-9]{3}[a-f0-9]{3}?\|rgba?\([0-9, ]+\))$/i
212	];
213	}
214	if (semantics.font.background) {	43✔
215	allowedStyles['background-color'] = [	29✔
216	/^(#[a-f0-9]{3}[a-f0-9]{3}?\|rgba?\([0-9, ]+\))$/i
217	];
218	}
219	if (semantics.font.spacing) {	43!
220	// tslint:disable-next-line: no-string-literal
NEW 221	allowedStyles['spacing'] = [/^[0-9.]+(em\|px\|%)$/i];	×
222	}
223	if (semantics.font.height) {	43!
NEW 224	allowedStyles['line-height'] = [/^[0-9.]+(em\|px\|%)$/i];	×
225	}
226	}
227
228	log.debug('Filtering out disallowed HTML tags');	1,135✔
229
230	newText = sanitizeHtml(newText, {	1,135✔
231	allowedTags,
232	allowedAttributes: {
233	'*': ['style'], // styles are filtered, so we can allow this
234	a: ['href', 'hreflang', 'media', 'rel', 'target'],
235	td: ['colspan', 'rowspan', 'headers'],
236	th: ['colspan', 'rowspan', 'headers', 'scope'],
237	ol: ['start', 'reversed'],
238	table: ['summary']
239	},
240	allowedStyles: { '*': allowedStyles }
241	});
242	} else {
243	log.debug('Filtering out all HTML tags');	4,418✔
244	// Escape all HTML tags if the field doesn't allow HTML in general.
245	newText = escape(newText);	4,418✔
246	}
247
248	// Check if string has the required length
249	if (semantics.maxLength && newText.length > semantics.maxLength) {	5,553✔
250	log.debug('Clipping string that is too long');	1✔
251	newText = newText.substring(0, semantics.maxLength);	1✔
252	}
253
254	// Check if string conforms to regexp (if given and if set to optional)
255	if (newText !== '' && semantics.optional && semantics.regexp) {	5,553✔
256	const regexp = new RegExp(	16✔
257	semantics.regexp.pattern,
258	semantics.regexp.modifiers
259	);
260	if (!regexp.test(newText)) {	16✔
261	log.debug(	1✔
262	`Provided string does not conform to regexp in semantics (value: ${newText}, regexp: ${semantics.regexp})`
263	);
264	newText = '';	1✔
265	}
266	}
267
268	parentParams[semantics.name] = newText;	5,553✔
269	}
270	}	25✔

Lumieducation / H5P-Nodejs-library / b3cffba5-e159-4c1b-9c4b-13e5d8490b0e

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous