Skip to content

feat(javascript): limit the depth of deserialize & serialize #3382

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.

Sign up for GitHub

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
chaokunyang merged 8 commits into from
Mar 11, 2026
Merged

feat(javascript): limit the depth of deserialize & serialize #3382

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
91d2795
limit the depth of deserialize & serialize
miantalha45 Feb 20, 2026
37d2d6c
Fix liniting errors
miantalha45 Feb 20, 2026
f88486f
refactor(javascript): optimize depth limiting by removing try/finally…
miantalha45 Feb 20, 2026
34e21dd
add decReadDepth in serializer.ts
miantalha45 Feb 21, 2026
f926cc1
fix(javascript): enforce depth limit on nested reads
chaokunyang Mar 11, 2026
94cb9ee
refactor(javascript): keep depth helpers internal
chaokunyang Mar 11, 2026
34d17ce
style(javascript): rename min depth constant
chaokunyang Mar 11, 2026
90557d2
style(javascript): rename default depth constant
chaokunyang Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions javascript/packages/fory/lib/fory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,15 @@ export default class {
anySerializer: Serializer;
typeMeta = TypeMeta;
config: Config;
depth = 0;
maxDepth: number;

constructor(config?: Partial<Config>) {
this.config = this.initConfig(config);
this.maxDepth = config?.maxDepth ?? 50;
if (this.maxDepth < 2) {
throw new Error(`maxDepth must be >= 2 but got ${this.maxDepth}`);
}
this.binaryReader = new BinaryReader(this.config);
this.binaryWriter = new BinaryWriter(this.config);
this.referenceResolver = new ReferenceResolver(this.binaryReader);
Expand All @@ -57,6 +63,7 @@ export default class {
return {
refTracking: config?.refTracking !== null ? Boolean(config?.refTracking) : null,
useSliceString: Boolean(config?.useSliceString),
maxDepth: config?.maxDepth,
hooks: config?.hooks || {},
compatible: Boolean(config?.compatible),
};
Expand All @@ -66,6 +73,20 @@ export default class {
return this.config.compatible === true;
}

incReadDepth(): void {
this.depth++;
if (this.depth > this.maxDepth) {
throw new Error(
`Deserialization depth limit exceeded: ${this.depth} > ${this.maxDepth}. `
+ "The data may be malicious, or increase maxDepth if needed."
);
}
}

decReadDepth(): void {
this.depth--;
}

registerSerializer<T>(constructor: new () => T, customSerializer: CustomSerializer<T>): {
serializer: Serializer;
serialize(data: InputType<T> | null): PlatformBuffer;
Expand Down Expand Up @@ -145,6 +166,7 @@ export default class {
this.binaryReader.reset(bytes);
this.typeMetaResolver.reset();
this.metaStringResolver.reset();
this.depth = 0;
const bitmap = this.binaryReader.readUint8();
if ((bitmap & ConfigFlags.isNullFlag) === ConfigFlags.isNullFlag) {
return null;
Expand Down
16 changes: 13 additions & 3 deletions javascript/packages/fory/lib/gen/serializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,13 @@ export abstract class BaseSerializerGenerator implements SerializerGenerator {

readNoRef(assignStmt: (v: string) => string, refState: string): string {
return `
${this.readTypeInfo()}
${this.read(assignStmt, refState)};
fory.incReadDepth();
try {
Copy link
Copy Markdown
Collaborator

@chaokunyang chaokunyang Feb 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the try finally will introduce extra cost. Please remove try finally, and refactor serialize/deserializae entry point, create resetWrite/resetRead methods, move code like:

    this.referenceResolver.resetRead();
    this.binaryReader.reset(bytes);
    this.typeMetaResolver.resetRead();
    this.metaStringResolver.resetRead();

into resetRead()

and move smililiar methods into resetWrite.

Then set depth to 0 in resetRead() and resetWrite.

Current referenceResolver/typeMetaResolver/metaStringResolver may lack some such methods, please take java as referencev and add to javascript

Copy link
Copy Markdown
Contributor Author

@miantalha45 miantalha45 Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

${this.readTypeInfo()}
${this.read(assignStmt, refState)};
} finally {
fory.decReadDepth();
}
`;
}

Expand All @@ -211,7 +216,12 @@ export abstract class BaseSerializerGenerator implements SerializerGenerator {
switch (${refFlag}) {
case ${RefFlags.NotNullValueFlag}:
case ${RefFlags.RefValueFlag}:
${this.read(assignStmt, `${refFlag} === ${RefFlags.RefValueFlag}`)}
fory.incReadDepth();
try {
${this.read(assignStmt, `${refFlag} === ${RefFlags.RefValueFlag}`)}
} finally {
fory.decReadDepth();
}
break;
case ${RefFlags.RefFlag}:
${assignStmt(this.builder.referenceResolver.getReadObject(this.builder.reader.readVarUInt32()))}
Expand Down
1 change: 1 addition & 0 deletions javascript/packages/fory/lib/type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ export interface Config {
hps?: Hps;
refTracking: boolean | null;
useSliceString: boolean;
maxDepth?: number;
hooks: {
afterCodeGenerated?: (code: string) => string;
};
Expand Down
Loading
Loading