Skip to content

DX | 24-02-2025 | Release #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 25, 2025
Merged
Prev Previous commit
Next Next commit
fix(attributes): sanitize keys and values to prevent html injection
  • Loading branch information
harshithad0703 committed Feb 19, 2025
commit 3d27ee2e1c6df2289f3614807bcf9fe7bd2e1b3e
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 = ['"', "'", '>','<', '/', '='];