Skip to content

Commit

Permalink
Also guard against entity expansion attacks in attribute values (#149)
Browse files Browse the repository at this point in the history
Looks like I missed this in the earlier implementation. This refactors the implementation a bit into a shape that also applies to any entities expanded as part of normalizing attribute values, both normal and default.
  • Loading branch information
bwrrp authored Dec 21, 2023
1 parent 527e54b commit a4f5c76
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 32 deletions.
55 changes: 55 additions & 0 deletions src/dom-parsing/EntityExpansionGuard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { EntityRefEvent } from './parserEvents';
import { throwErrorWithContext } from './parsingAlgorithms';

/*
* Guard against entity expansion attacks by keeping track of the initial input
* length vs. the expanded input length. The latter includes the length of the
* replacement text for each processed entity reference. An attack is likely if
* the ratio between the two exceeds the maximum amplification factor AND the
* expanded input length exceeds a threshold. This approach and defaults are
* taken from libexpat's billion laughs attack protection.
*/
export default class EntityExpansionGuard {
private readonly _initialInputLength: number;

private _expandedInputLength: number;

private readonly _entityExpansionThreshold: number;

private readonly _entityExpansionMaxAmplification: number;

private _topLevelEntityRef: EntityRefEvent | null = null;

private _depth = 0;

public constructor(
initialInputLength: number,
entityExpansionThreshold: number,
entityExpansionMaxAmplification: number
) {
this._initialInputLength = initialInputLength;
this._expandedInputLength = initialInputLength;
this._entityExpansionThreshold = entityExpansionThreshold;
this._entityExpansionMaxAmplification = entityExpansionMaxAmplification;
}

public enter(event: EntityRefEvent, replacementTextLength: number): void {
const topLevelEntityRef = this._topLevelEntityRef ?? event;
this._expandedInputLength += replacementTextLength;
if (this._expandedInputLength > this._entityExpansionThreshold) {
const amplification = this._expandedInputLength / this._initialInputLength;
if (amplification > this._entityExpansionMaxAmplification) {
throwErrorWithContext('too much entity expansion', topLevelEntityRef);
}
}
this._topLevelEntityRef = topLevelEntityRef;
this._depth += 1;
}

public exit(): void {
this._depth -= 1;
if (this._depth === 0) {
this._topLevelEntityRef = null;
}
}
}
57 changes: 25 additions & 32 deletions src/dom-parsing/parsingAlgorithms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { insertNode } from '../util/mutationAlgorithms';
import { XMLNS_NAMESPACE, XML_NAMESPACE } from '../util/namespaceHelpers';
import { isElement } from '../util/NodeType';
import { getNodeDocument } from '../util/treeHelpers';
import EntityExpansionGuard from './EntityExpansionGuard';
import {
CompleteChars,
CompleteName,
Expand Down Expand Up @@ -343,7 +344,8 @@ function normalizeAndIncludeEntities(
normalized: string[],
value: AttValueEvent[],
dtd: Dtd | null,
ancestorEntities: string[] | null
ancestorEntities: string[] | null,
expansionGuard: EntityExpansionGuard
) {
for (const event of value) {
if (typeof event === 'string') {
Expand Down Expand Up @@ -372,6 +374,7 @@ function normalizeAndIncludeEntities(
event
);
}
expansionGuard.enter(event, replacementText.length);
const result = EntityReplacementTextInLiteral(replacementText, 0);
if (!result.success) {
throwParseError(
Expand All @@ -386,18 +389,21 @@ function normalizeAndIncludeEntities(
normalized,
result.value,
dtd,
ancestorEntities ? [event.name, ...ancestorEntities] : [event.name]
ancestorEntities ? [event.name, ...ancestorEntities] : [event.name],
expansionGuard
);
expansionGuard.exit();
}
}

function normalizeAttributeValue(
value: AttValueEvent[],
attDef: AttDefEvent | undefined,
dtd: Dtd | null
dtd: Dtd | null,
expansionGuard: EntityExpansionGuard
): string {
const normalized: string[] = [];
normalizeAndIncludeEntities(normalized, value, dtd, null);
normalizeAndIncludeEntities(normalized, value, dtd, null, expansionGuard);
if (attDef && !attDef.isCData) {
return normalized
.join('')
Expand Down Expand Up @@ -506,7 +512,8 @@ class Namespaces {
event: STagEvent | EmptyElemTagEvent,
attlist: Map<string, AttDefEvent> | undefined,
dtd: Dtd | null,
qualifiedNameCache: QualifiedNameCache
qualifiedNameCache: QualifiedNameCache,
expansionGuard: EntityExpansionGuard
): Namespaces {
let ns = parent;
let hasDeclarations = false;
Expand Down Expand Up @@ -558,7 +565,7 @@ class Namespaces {
localName === 'xmlns' &&
(!hasDeclarations || !ns._byPrefix.has(null))
) {
const namespace = normalizeAttributeValue(value, def, dtd) || null;
const namespace = normalizeAttributeValue(value, def, dtd, expansionGuard) || null;
add(null, namespace, nameEvent);
} else if (prefix === 'xmlns' && (!hasDeclarations || !ns._byPrefix.has(localName))) {
if (localName === 'xmlns') {
Expand All @@ -567,7 +574,7 @@ class Namespaces {
nameEvent
);
}
const namespace = normalizeAttributeValue(value, def, dtd) || null;
const namespace = normalizeAttributeValue(value, def, dtd, expansionGuard) || null;
add(localName, namespace, nameEvent);
}
};
Expand Down Expand Up @@ -719,15 +726,11 @@ export function parseXml(
input = input.replace(/^\ufeff/, '');
input = normalizeLineEndings(input);

// Guard against entity expansion attacks by keeping track of the initial input length vs. the
// expanded input length. The latter includes the length of the replacement text for each
// processed entity reference. An attack is likely if the ratio between the two exceeds the
// maximum amplification factor AND the expanded input length exceeds a threshold. This approach
// and defaults are taken from libexpat's billion laughs attack protection.
const initialInputLength = input.length;
let expandedInputLength = initialInputLength;
let topLevelEntityRef: EntityRefEvent | null = null;

const expansionGuard = new EntityExpansionGuard(
input.length,
entityExpansionThreshold,
entityExpansionMaxAmplification
);
let entityContext: EntityContext | null = {
parent: null,
entity: null,
Expand Down Expand Up @@ -778,16 +781,7 @@ export function parseXml(
event
);
}
if (topLevelEntityRef === null) {
topLevelEntityRef = event;
}
expandedInputLength += replacementText.length;
if (expandedInputLength > entityExpansionThreshold) {
const amplification = expandedInputLength / initialInputLength;
if (amplification > entityExpansionMaxAmplification) {
throwErrorWithContext('too much entity expansion', topLevelEntityRef);
}
}
expansionGuard.enter(event, replacementText.length);
domContext = {
parent: domContext,
root: domContext.root,
Expand Down Expand Up @@ -861,7 +855,8 @@ export function parseXml(
event,
attlist,
dtd,
qualifiedNameCache
qualifiedNameCache,
expansionGuard
);
const { prefix, localName } = splitQualifiedName(
event.name,
Expand All @@ -888,7 +883,7 @@ export function parseXml(
namespace,
prefix,
localName,
normalizeAttributeValue(attr.value, def, dtd),
normalizeAttributeValue(attr.value, def, dtd, expansionGuard),
element
);
appendAttribute(attrNode, element, true);
Expand Down Expand Up @@ -917,7 +912,7 @@ export function parseXml(
namespace,
prefix,
localName,
normalizeAttributeValue(def.value, attr, dtd),
normalizeAttributeValue(def.value, attr, dtd, expansionGuard),
element
);
appendAttribute(attrNode, element, true);
Expand Down Expand Up @@ -981,10 +976,8 @@ export function parseXml(

entityContext = entityContext.parent;
if (entityContext) {
expansionGuard.exit();
domContext = domContext.parent!;
if (entityContext.entity === null) {
topLevelEntityRef = null;
}
}
}

Expand Down
78 changes: 78 additions & 0 deletions test/dom-parsing/parseXmlDocument.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,84 @@ describe('parseXmlDocument', () => {
`);
});

it('protects against entity expansion attacks in attribute values', () => {
// Each level expands to 10 times the next, leading to 10^10 expansions
const xml = `<!DOCTYPE root [
<!ENTITY one "&two;&two;&two;&two;&two;&two;&two;&two;&two;&two;&two;">
<!ENTITY two "&three;&three;&three;&three;&three;&three;&three;&three;&three;&three;&three;">
<!ENTITY three "&four;&four;&four;&four;&four;&four;&four;&four;&four;&four;&four;">
<!ENTITY four "&five;&five;&five;&five;&five;&five;&five;&five;&five;&five;&five;">
<!ENTITY five "&six;&six;&six;&six;&six;&six;&six;&six;&six;&six;&six;">
<!ENTITY six "&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;">
<!ENTITY seven "&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;">
<!ENTITY eight "&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;">
<!ENTITY nine "&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;">
<!ENTITY ten "a">
]><root attr="&one;"/>`;
expect(() => {
console.log(slimdom.parseXmlDocument(xml));
}).toThrowErrorMatchingInlineSnapshot(`
"too much entity expansion
At line 12, character 17:
]><root attr="&one;"/>
^^^^^"
`);
});

it('protects against entity expansion attacks in default attribute values', () => {
// Each level expands to 10 times the next, leading to 10^10 expansions
const xml = `<!DOCTYPE root [
<!ENTITY one "&two;&two;&two;&two;&two;&two;&two;&two;&two;&two;&two;">
<!ENTITY two "&three;&three;&three;&three;&three;&three;&three;&three;&three;&three;&three;">
<!ENTITY three "&four;&four;&four;&four;&four;&four;&four;&four;&four;&four;&four;">
<!ENTITY four "&five;&five;&five;&five;&five;&five;&five;&five;&five;&five;&five;">
<!ENTITY five "&six;&six;&six;&six;&six;&six;&six;&six;&six;&six;&six;">
<!ENTITY six "&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;">
<!ENTITY seven "&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;">
<!ENTITY eight "&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;">
<!ENTITY nine "&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;">
<!ENTITY ten "a">
<!ATTLIST root attr CDATA "&one;">
]><root/>`;
expect(() => {
console.log(slimdom.parseXmlDocument(xml));
}).toThrowErrorMatchingInlineSnapshot(`
"too much entity expansion
At line 12, character 31:
<!ATTLIST root attr CDATA "&one;">
^^^^^"
`);
});

it('protects against entity expansion attacks in default attributes on elements generated from an entity', () => {
// Each level expands to 10 times the next, leading to 10^10 expansions
const xml = `<!DOCTYPE root [
<!ENTITY one "&two;&two;&two;&two;&two;&two;&two;&two;&two;&two;&two;">
<!ENTITY two "&three;&three;&three;&three;&three;&three;&three;&three;&three;&three;&three;">
<!ENTITY three "&four;&four;&four;&four;&four;&four;&four;&four;&four;&four;&four;">
<!ENTITY four "&five;&five;&five;&five;&five;&five;&five;&five;&five;&five;&five;">
<!ENTITY five "&six;&six;&six;&six;&six;&six;&six;&six;&six;&six;&six;">
<!ENTITY six "&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;&seven;">
<!ENTITY seven "&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;&eight;">
<!ENTITY eight "&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;&nine;">
<!ENTITY nine "&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;&ten;">
<!ENTITY ten "a">
<!ENTITY ele "&#60;element/>">
<!ATTLIST element attr CDATA "&one;">
]><root>&ele;</root>`;
expect(() => {
console.log(slimdom.parseXmlDocument(xml));
}).toThrowErrorMatchingInlineSnapshot(`
"too much entity expansion
At line 14, character 11:
]><root>&ele;</root>
^^^^^"
`);
});

it('can optionally treat CDATA sections as text', () => {
const xml = '<xml>before<![CDATA[inside]]>after</xml>';
const doc1 = slimdom.parseXmlDocument(xml);
Expand Down

0 comments on commit a4f5c76

Please sign in to comment.