Skip to content

Commit

Permalink
fix(attributes): sanitize keys and values to prevent html injection
Browse files Browse the repository at this point in the history
  • Loading branch information
harshithad0703 committed Feb 19, 2025
1 parent 1905567 commit 3d27ee2
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 23 deletions.
75 changes: 75 additions & 0 deletions __test__/attributes-to-string.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
"<ls": "\"></p><h1>test</h1><p class=\"",
"\"></p><h1>test</h1><p class=\"": 1
} as Attributes;

const resultString = attributeToString(attr);

expect(resultString).toEqual(' style=\"text-align:left; \" rows=\"4\" cols=\"2\" colWidths=\"250, 250\" &lt;ls=\"&quot;&gt;&lt;/p&gt;&lt;h1&gt;test&lt;/h1&gt;&lt;p class=&quot;\"')
done();
});
it('Should handle object attribute values correctly', done => {
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 <script>alert(xss)</script>"
} as Attributes;

const resultString = attributeToString(attr);

expect(resultString).toEqual(' title="This &amp; That &gt; Those &lt; Them &quot;Quoted&quot;" description="Hello &lt;script&gt;alert(xss)&lt;/script&gt;"');
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 = {
"<script>alert('key')</script>": "test",
"safeKey": "<script>alert(xss)</script>"
} as Attributes;

const resultString = attributeToString(attr);

expect(resultString).toEqual(' safeKey="&lt;script&gt;alert(xss)&lt;/script&gt;"');
done();
});
})
42 changes: 22 additions & 20 deletions src/Models/metadata-model.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions src/helper/enumerate-entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -159,9 +159,12 @@ function nodeToHTML(
}
}

function escapeHtml(text: string): string {
export function replaceHtmlEntities(text: string): string {
return text
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
}
.replace(/"/g, '&quot;')
}

export const forbiddenAttrChars = ['"', "'", '>','<', '/', '='];

0 comments on commit 3d27ee2

Please sign in to comment.