From 3d27ee2e1c6df2289f3614807bcf9fe7bd2e1b3e Mon Sep 17 00:00:00 2001 From: "harshitha.d" Date: Wed, 19 Feb 2025 12:23:54 +0530 Subject: [PATCH] fix(attributes): sanitize keys and values to prevent html injection --- __test__/attributes-to-string.test.ts | 75 +++++++++++++++++++++++++++ src/Models/metadata-model.ts | 42 ++++++++------- src/helper/enumerate-entries.ts | 9 ++-- 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/__test__/attributes-to-string.test.ts b/__test__/attributes-to-string.test.ts index 4b8173a..ca64f53 100644 --- a/__test__/attributes-to-string.test.ts +++ b/__test__/attributes-to-string.test.ts @@ -49,5 +49,80 @@ describe('Attributes to String', () => { expect(resultString).toEqual(' style="text-align:left; " rows="4" cols="2" colWidths="250, 250"') done() }) + it('Should rignore attributes with forbidden characters in keys and values', done => { + const attr = { + "style": { + "text-align": "left" + }, + "rows": 4, + "cols": 2, + "colWidths": [250, 250], + "

test

test

{ + const attr = { + "style": { + "color": "red", + "font-size": "14px" + } + } as Attributes; + + const resultString = attributeToString(attr); + + expect(resultString).toEqual(' style="color:red; font-size:14px; "'); + done(); + }); + it('Should convert arrays into comma-separated values', done => { + const attr = { + "data-values": [10, 20, 30] + } as Attributes; + + const resultString = attributeToString(attr); + + expect(resultString).toEqual(' data-values="10, 20, 30"'); + done(); + }); + it('Should handle special characters in values properly', done => { + const attr = { + "title": 'This & That > Those < Them "Quoted"', + "description": "Hello " + } as Attributes; + + const resultString = attributeToString(attr); + + expect(resultString).toEqual(' title="This & That > Those < Them "Quoted"" description="Hello <script>alert(xss)</script>"'); + done(); + }); + + it('Should handle mixed types of values properly', done => { + const attr = { + "rows": 5, + "isEnabled": true, + "ids": [101, 102], + "style": { "margin": "10px", "padding": "5px" } + } as Attributes; + + const resultString = attributeToString(attr); + + expect(resultString).toEqual(' rows="5" isEnabled="true" ids="101, 102" style="margin:10px; padding:5px; "'); + done(); + }); + it('Should sanitize both keys and values to prevent HTML injection', done => { + const attr = { + "": "test", + "safeKey": "" + } as Attributes; + + const resultString = attributeToString(attr); + expect(resultString).toEqual(' safeKey="<script>alert(xss)</script>"'); + done(); + }); }) \ No newline at end of file diff --git a/src/Models/metadata-model.ts b/src/Models/metadata-model.ts index 8d8a354..dd95c3b 100644 --- a/src/Models/metadata-model.ts +++ b/src/Models/metadata-model.ts @@ -1,5 +1,7 @@ import StyleType from '../embedded-types/style-type'; import TextNode from '../nodes/text-node'; +import { replaceHtmlEntities, forbiddenAttrChars } from '../helper/enumerate-entries'; + export interface Metadata { text: string; attributes: Attributes; @@ -58,30 +60,30 @@ export function attributeToString(attributes: Attributes): string { let result = ''; for (const key in attributes) { if (Object.prototype.hasOwnProperty.call(attributes, key)) { - let element = attributes[key]; - if (element instanceof Array) { - let elementString = ''; - let isFirst = true; - element.forEach((value) => { - if (isFirst) { - elementString += `${value}`; - isFirst = false; - } else { - elementString += `, ${value}`; - } - }); - element = elementString; - } else if (typeof element === 'object') { + // Sanitize the key to prevent HTML injection + const sanitizedKey = replaceHtmlEntities(key); + + // Skip keys that contain forbidden characters (even after sanitization) + if (forbiddenAttrChars.some(char => sanitizedKey.includes(char))) { + continue; + } + let value = attributes[key]; + if (Array.isArray(value)) { + value = value.join(', '); + } else if (typeof value === 'object') { let elementString = ''; - for (const elementKey in element) { - if (Object.prototype.hasOwnProperty.call(element, elementKey)) { - const value = element[elementKey]; - elementString += `${elementKey}:${value}; `; + for (const subKey in value) { + if (Object.prototype.hasOwnProperty.call(value, subKey)) { + const subValue = value[subKey]; + if (subValue != null && subValue !== '') { + elementString += `${replaceHtmlEntities(subKey)}:${replaceHtmlEntities(String(subValue))}; `; + } } } - element = elementString; + value = elementString; } - result += ` ${key}="${element}"`; + // Sanitize the value to prevent HTML injection + result += ` ${sanitizedKey}="${replaceHtmlEntities(String(value))}"`; } } return result; diff --git a/src/helper/enumerate-entries.ts b/src/helper/enumerate-entries.ts index c23e5b6..d76df3b 100644 --- a/src/helper/enumerate-entries.ts +++ b/src/helper/enumerate-entries.ts @@ -42,7 +42,7 @@ export function enumerateContents( } export function textNodeToHTML(node: TextNode, renderOption: RenderOption): string { - let text = escapeHtml(node.text); + let text = replaceHtmlEntities(node.text); if (node.classname || node.id) { text = (renderOption[MarkType.CLASSNAME_OR_ID] as RenderMark)(text, node.classname, node.id); } @@ -159,9 +159,12 @@ function nodeToHTML( } } -function escapeHtml(text: string): string { +export function replaceHtmlEntities(text: string): string { return text .replace(/&/g, '&') .replace(//g, '>') -} \ No newline at end of file + .replace(/"/g, '"') +} + +export const forbiddenAttrChars = ['"', "'", '>','<', '/', '=']; \ No newline at end of file