Skip to content

Commit 88d0936

Browse files
committed
apply all fixes from v5
- support maxEntityCount - support onDangerousProperty - support maxNestedTags - handle prototype pollution - fix incorrect entity name replacement - fix incorrect condition for entity expansion -
1 parent d4eb6b4 commit 88d0936

File tree

5 files changed

+157
-19
lines changed

5 files changed

+157
-19
lines changed

src/fxp.d.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ export type ProcessEntitiesOptions = {
3434
*/
3535
maxExpandedLength?: number;
3636

37+
/**
38+
* Maximum number of entities allowed in the XML
39+
*
40+
* Defaults to `100`
41+
*/
42+
maxEntityCount?: number;
43+
3744
/**
3845
* Array of tag names where entity replacement is allowed.
3946
* If null, entities are replaced in all tags.
@@ -292,6 +299,16 @@ export type X2jOptions = {
292299
* Defaults to `true`
293300
*/
294301
strictReservedNames?: boolean;
302+
303+
/**
304+
* Function to sanitize dangerous property names
305+
*
306+
* @param name - The name of the property
307+
* @returns {string} The sanitized name
308+
*
309+
* Defaults to `(name) => __name`
310+
*/
311+
onDangerousProperty?: (name: string) => string;
295312
};
296313

297314

@@ -469,6 +486,13 @@ export type XmlBuilderOptions = {
469486

470487

471488
oneListGroup?: boolean;
489+
490+
/**
491+
* Maximum number of nested tags
492+
*
493+
* Defaults to `100`
494+
*/
495+
maxNestedTags?: number;
472496
};
473497

474498
type ESchema = string | object | Array<string | object>;

src/util.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const nameChar = nameStartChar + '\\-.\\d\\u00B7\\u0300-\\u036F\\u203F-\\u2040';
55
const nameRegexp = '[' + nameStartChar + '][' + nameChar + ']*'
66
const regexName = new RegExp('^' + nameRegexp + '$');
77

8-
const getAllMatches = function(string, regex) {
8+
const getAllMatches = function (string, regex) {
99
const matches = [];
1010
let match = regex.exec(string);
1111
while (match) {
@@ -21,16 +21,16 @@ const getAllMatches = function(string, regex) {
2121
return matches;
2222
};
2323

24-
const isName = function(string) {
24+
const isName = function (string) {
2525
const match = regexName.exec(string);
2626
return !(match === null || typeof match === 'undefined');
2727
};
2828

29-
exports.isExist = function(v) {
29+
exports.isExist = function (v) {
3030
return typeof v !== 'undefined';
3131
};
3232

33-
exports.isEmptyObject = function(obj) {
33+
exports.isEmptyObject = function (obj) {
3434
return Object.keys(obj).length === 0;
3535
};
3636

@@ -39,13 +39,13 @@ exports.isEmptyObject = function(obj) {
3939
* @param {*} target
4040
* @param {*} a
4141
*/
42-
exports.merge = function(target, a, arrayMode) {
42+
exports.merge = function (target, a, arrayMode) {
4343
if (a) {
4444
const keys = Object.keys(a); // will return an array of own properties
4545
const len = keys.length; //don't make it inline
4646
for (let i = 0; i < len; i++) {
4747
if (arrayMode === 'strict') {
48-
target[keys[i]] = [ a[keys[i]] ];
48+
target[keys[i]] = [a[keys[i]]];
4949
} else {
5050
target[keys[i]] = a[keys[i]];
5151
}
@@ -56,17 +56,34 @@ exports.merge = function(target, a, arrayMode) {
5656
return Object.assign(b,a);
5757
} */
5858

59-
exports.getValue = function(v) {
59+
exports.getValue = function (v) {
6060
if (exports.isExist(v)) {
6161
return v;
6262
} else {
6363
return '';
6464
}
6565
};
6666

67-
// const fakeCall = function(a) {return a;};
68-
// const fakeCallNoReturn = function() {};
67+
/**
68+
* Dangerous property names that could lead to prototype pollution or security issues
69+
*/
70+
const DANGEROUS_PROPERTY_NAMES = [
71+
// '__proto__',
72+
// 'constructor',
73+
// 'prototype',
74+
'hasOwnProperty',
75+
'toString',
76+
'valueOf',
77+
'__defineGetter__',
78+
'__defineSetter__',
79+
'__lookupGetter__',
80+
'__lookupSetter__'
81+
];
82+
83+
const criticalProperties = ["__proto__", "constructor", "prototype"];
6984

7085
exports.isName = isName;
7186
exports.getAllMatches = getAllMatches;
7287
exports.nameRegexp = nameRegexp;
88+
exports.DANGEROUS_PROPERTY_NAMES = DANGEROUS_PROPERTY_NAMES;
89+
exports.criticalProperties = criticalProperties;

src/xmlparser/DocTypeReader.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class DocTypeReader {
88

99
readDocType(xmlData, i) {
1010
const entities = Object.create(null);
11+
let entityCount = 0;
1112

1213
if (xmlData[i + 3] === 'O' &&
1314
xmlData[i + 4] === 'C' &&
@@ -28,11 +29,20 @@ class DocTypeReader {
2829
let entityName, val;
2930
[entityName, val, i] = this.readEntityExp(xmlData, i + 1, this.suppressValidationErr);
3031
if (val.indexOf("&") === -1) { //Parameter entities are not supported
31-
const escaped = entityName.replace(/[.\-+*:]/g, '\\.');
32+
if (this.options.enabled !== false &&
33+
this.options.maxEntityCount != null &&
34+
entityCount >= this.options.maxEntityCount) {
35+
throw new Error(
36+
`Entity count (${entityCount + 1}) exceeds maximum allowed (${this.options.maxEntityCount})`
37+
);
38+
}
39+
//const escaped = entityName.replace(/[.\-+*:]/g, '\\.');
40+
const escaped = entityName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
3241
entities[entityName] = {
3342
regx: RegExp(`&${escaped};`, "g"),
3443
val: val
3544
};
45+
entityCount++;
3646
}
3747
} else if (hasBody && hasSeq(xmlData, "!ELEMENT", i)) {
3848
i += 8; //Not supported
@@ -122,7 +132,7 @@ class DocTypeReader {
122132

123133
// Validate entity size
124134
if (this.options.enabled !== false &&
125-
this.options.maxEntitySize &&
135+
this.options.maxEntitySize != null &&
126136
entityValue.length > this.options.maxEntitySize) {
127137
throw new Error(
128138
`Entity "${entityName}" size (${entityValue.length}) exceeds maximum allowed size (${this.options.maxEntitySize})`

src/xmlparser/OptionsBuilder.js

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11

2+
const { DANGEROUS_PROPERTY_NAMES, criticalProperties } = require("../util");
3+
4+
const defaultOnDangerousProperty = (name) => {
5+
if (DANGEROUS_PROPERTY_NAMES.includes(name)) {
6+
return "__" + name;
7+
}
8+
return name;
9+
};
210
const defaultOptions = {
311
preserveOrder: false,
412
attributeNamePrefix: '@_',
@@ -41,7 +49,32 @@ const defaultOptions = {
4149
captureMetaData: false,
4250
maxNestedTags: 100,
4351
strictReservedNames: true,
52+
onDangerousProperty: defaultOnDangerousProperty
4453
};
54+
/**
55+
* Validates that a property name is safe to use
56+
* @param {string} propertyName - The property name to validate
57+
* @param {string} optionName - The option field name (for error message)
58+
* @throws {Error} If property name is dangerous
59+
*/
60+
function validatePropertyName(propertyName, optionName) {
61+
if (typeof propertyName !== 'string') {
62+
return; // Only validate string property names
63+
}
64+
65+
const normalized = propertyName.toLowerCase();
66+
if (DANGEROUS_PROPERTY_NAMES.some(dangerous => normalized === dangerous.toLowerCase())) {
67+
throw new Error(
68+
`[SECURITY] Invalid ${optionName}: "${propertyName}" is a reserved JavaScript keyword that could cause prototype pollution`
69+
);
70+
}
71+
72+
if (criticalProperties.some(dangerous => normalized === dangerous.toLowerCase())) {
73+
throw new Error(
74+
`[SECURITY] Invalid ${optionName}: "${propertyName}" is a reserved JavaScript keyword that could cause prototype pollution`
75+
);
76+
}
77+
}
4578

4679
/**
4780
* Normalizes processEntities option for backward compatibility
@@ -65,11 +98,12 @@ function normalizeProcessEntities(value) {
6598
// Object config - merge with defaults
6699
if (typeof value === 'object' && value !== null) {
67100
return {
68-
enabled: value.enabled !== false, // default true if not specified
69-
maxEntitySize: value.maxEntitySize ?? 10000,
70-
maxExpansionDepth: value.maxExpansionDepth ?? 10,
71-
maxTotalExpansions: value.maxTotalExpansions ?? 1000,
72-
maxExpandedLength: value.maxExpandedLength ?? 100000,
101+
enabled: value.enabled !== false,
102+
maxEntitySize: Math.max(1, value.maxEntitySize ?? 10000),
103+
maxExpansionDepth: Math.max(1, value.maxExpansionDepth ?? 10),
104+
maxTotalExpansions: Math.max(1, value.maxTotalExpansions ?? 1000),
105+
maxExpandedLength: Math.max(1, value.maxExpandedLength ?? 100000),
106+
maxEntityCount: Math.max(1, value.maxEntityCount ?? 100),
73107
allowedTags: value.allowedTags ?? null,
74108
tagFilter: value.tagFilter ?? null
75109
};
@@ -82,6 +116,26 @@ function normalizeProcessEntities(value) {
82116
const buildOptions = function (options) {
83117
const built = Object.assign({}, defaultOptions, options);
84118

119+
120+
// Validate property names to prevent prototype pollution
121+
const propertyNameOptions = [
122+
{ value: built.attributeNamePrefix, name: 'attributeNamePrefix' },
123+
{ value: built.attributesGroupName, name: 'attributesGroupName' },
124+
{ value: built.textNodeName, name: 'textNodeName' },
125+
{ value: built.cdataPropName, name: 'cdataPropName' },
126+
{ value: built.commentPropName, name: 'commentPropName' }
127+
];
128+
129+
for (const { value, name } of propertyNameOptions) {
130+
if (value) {
131+
validatePropertyName(value, name);
132+
}
133+
}
134+
135+
if (built.onDangerousProperty === null) {
136+
built.onDangerousProperty = defaultOnDangerousProperty;
137+
}
138+
85139
// Always normalize processEntities for backward compatibility and validation
86140
built.processEntities = normalizeProcessEntities(built.processEntities);
87141
//console.debug(built.processEntities)

src/xmlparser/OrderedObjParser.js

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ function buildAttributesMap(attrStr, jPath, tagName) {
162162
if (this.options.transformAttributeName) {
163163
aName = this.options.transformAttributeName(aName);
164164
}
165-
if (aName === "__proto__") aName = "#__proto__";
165+
aName = sanitizeName(aName, this.options);
166166
if (oldVal !== undefined) {
167167
if (this.options.trimValues) {
168168
oldVal = oldVal.trim();
@@ -325,6 +325,8 @@ const parseXml = function (xmlData) {
325325
if (this.options.strictReservedNames &&
326326
(tagName === this.options.commentPropName
327327
|| tagName === this.options.cdataPropName
328+
|| tagName === this.options.textNodeName
329+
|| tagName === this.options.attributesGroupName
328330
)) {
329331
throw new Error(`Invalid tag name: ${tagName}`);
330332
}
@@ -523,16 +525,37 @@ const replaceEntitiesValue = function (val, tagName, jPath) {
523525
if (val.indexOf('&') === -1) return val; // Early exit
524526

525527
// Replace standard entities
526-
for (let entityName in this.lastEntities) {
528+
for (const entityName of Object.keys(this.lastEntities)) {
527529
const entity = this.lastEntities[entityName];
530+
const matches = val.match(entity.regex);
531+
if (matches) {
532+
this.entityExpansionCount += matches.length;
533+
if (entityConfig.maxTotalExpansions &&
534+
this.entityExpansionCount > entityConfig.maxTotalExpansions) {
535+
throw new Error(
536+
`Entity expansion limit exceeded: ${this.entityExpansionCount} > ${entityConfig.maxTotalExpansions}`
537+
);
538+
}
539+
}
528540
val = val.replace(entity.regex, entity.val);
529541
}
530542
if (val.indexOf('&') === -1) return val; // Early exit
531543

532544
// Replace HTML entities if enabled
533545
if (this.options.htmlEntities) {
534-
for (let entityName in this.htmlEntities) {
546+
for (const entityName of Object.keys(this.htmlEntities)) {
535547
const entity = this.htmlEntities[entityName];
548+
const matches = val.match(entity.regex);
549+
if (matches) {
550+
//console.log(matches);
551+
this.entityExpansionCount += matches.length;
552+
if (entityConfig.maxTotalExpansions &&
553+
this.entityExpansionCount > entityConfig.maxTotalExpansions) {
554+
throw new Error(
555+
`Entity expansion limit exceeded: ${this.entityExpansionCount} > ${entityConfig.maxTotalExpansions}`
556+
);
557+
}
558+
}
536559
val = val.replace(entity.regex, entity.val);
537560
}
538561
}
@@ -725,4 +748,14 @@ function fromCodePoint(str, base, prefix) {
725748
}
726749
}
727750

751+
function sanitizeName(name, options) {
752+
if (util.criticalProperties.includes(name)) {
753+
throw new Error(`[SECURITY] Invalid name: "${name}" is a reserved JavaScript keyword that could cause prototype pollution`);
754+
} else if (util.DANGEROUS_PROPERTY_NAMES.includes(name)) {
755+
return options.onDangerousProperty(name);
756+
}
757+
return name;
758+
}
759+
728760
module.exports = OrderedObjParser;
761+

0 commit comments

Comments
 (0)