Skip to content

Harden ASF parser against infinite loop and malformed headers #2624

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

Open
Borewit wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Harden ASF parser against infinite loop and malformed headers #2624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/ParseError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class UnsupportedFileTypeError extends makeParseError('UnsupportedFileTyp
}

// Concrete error class representing unexpected file content.
class UnexpectedFileContentError extends makeParseError('UnexpectedFileContentError') {
export class UnexpectedFileContentError extends makeParseError('UnexpectedFileContentError') {
public readonly fileType: string;

constructor(fileType: string, message: string) {
Expand Down
13 changes: 9 additions & 4 deletions lib/asf/AsfObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,12 @@ export const TopLevelHeaderObjectToken: IGetToken<IAsfTopLevelObjectHeader, Uint
len: 30,

get: (buf, off): IAsfTopLevelObjectHeader => {
const base = HeaderObjectToken.get(buf, off);

return {
objectId: AsfGuid.fromBin(buf, off),
objectSize: Number(Token.UINT64_LE.get(buf, off + 16)),
...base,
numberOfHeaderObjects: Token.UINT32_LE.get(buf, off + 24)
// Reserved: 2 bytes
// reserved: 2 bytes
};
Comment on lines 79 to 86
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

TopLevelHeaderObjectToken now reuses HeaderObjectToken.get, but it only enforces objectSize >= 24. The ASF top-level Header Object requires at least 30 bytes (GUID+size+numberOfHeaderObjects+reserved). Add a check that base.objectSize >= TopLevelHeaderObjectToken.len (30) to properly reject malformed headers in the 24..29 range.

Copilot uses AI. Check for mistakes.
}
};
Expand All @@ -95,10 +96,14 @@ export const HeaderObjectToken: IGetToken<IAsfObjectHeader, Uint8Array> = {
len: 24,

get: (buf, off): IAsfObjectHeader => {
return {
const header = {
objectId: AsfGuid.fromBin(buf, off),
objectSize: Number(Token.UINT64_LE.get(buf, off + 16))
};
if (header.objectSize < 24) {
throw new AsfContentParseError(`Invalid ASF header object size: ${header.objectSize}`);
}
return header;
}
};

Expand Down
10 changes: 4 additions & 6 deletions lib/asf/AsfParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import * as AsfObject from './AsfObject.js';
import { BasicParser } from '../common/BasicParser.js';
import { AsfContentParseError } from './AsfObject.js';


const debug = initDebug('music-metadata:parser:ASF');
const headerType = 'asf';

Expand All @@ -24,8 +23,10 @@ export class AsfParser extends BasicParser {

public async parse() {
const header = await this.tokenizer.readToken<AsfObject.IAsfTopLevelObjectHeader>(AsfObject.TopLevelHeaderObjectToken);
if (!header.objectId.equals(AsfGuid.HeaderObject)) {
throw new AsfContentParseError(`expected asf header; but was not found; got: ${header.objectId.str}`);
if (header.numberOfHeaderObjects > 10000) {
throw new AsfContentParseError(
`Unrealistic number of ASF header objects: ${header.numberOfHeaderObjects}`
);
}
await this.parseObjectHeader(header.numberOfHeaderObjects);
Comment on lines 24 to 31
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

parse() no longer verifies the top-level ASF GUID is AsfGuid.HeaderObject (the previous expected asf header check was removed). This can allow non-ASF input to be parsed as ASF, changing failure mode and potentially masking file-type issues. Consider restoring the GUID check (in addition to the new numberOfHeaderObjects sanity limit).

Copilot uses AI. Check for mistakes.
}
Comment on lines +29 to 32
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The parseObjectHeader(header.numberOfHeaderObjects) call inherits a subtle edge case from parseObjectHeader’s do { ... } while (--numberOfObjectHeaders) loop: if numberOfHeaderObjects is 0, the loop decrements into negative values (truthy) and becomes unbounded until EOF/error. Consider validating numberOfHeaderObjects >= 1 here (or rewriting the loop) so a 0 value cannot trigger excessive parsing/DoS behavior.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -110,9 +111,6 @@ export class AsfParser extends BasicParser {
// Parse common header of the ASF Object (3.1)
const header = await this.tokenizer.readToken<AsfObject.IAsfObjectHeader>(AsfObject.HeaderObjectToken);
const remaining = header.objectSize - AsfObject.HeaderObjectToken.len;
if (remaining < 0) {
throw new AsfContentParseError(`Invalid ASF header object size: ${header.objectSize}`);
}
// Parse data part of the ASF Object
switch (header.objectId.str) {

Expand Down
Binary file added test/samples/aiff/CWE-835-01.aiff
View file
Open in desktop
Binary file not shown.
Binary file added test/samples/asf/CWE-835-03.wma
View file
Open in desktop
Binary file not shown.
Binary file added test/samples/asf/max-numberOfObjectHeaders.wma
View file
Open in desktop
Binary file not shown.
16 changes: 15 additions & 1 deletion test/test-file-aiff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import path from 'node:path';
import { Parsers } from './metadata-parsers.js';
import { assert } from 'chai';
import { assert, expect } from 'chai';
import * as mm from '../lib/index.js';
import { samplePath} from './util.js';
import { UnexpectedFileContentError } from '../lib/index.js';

describe('Parse AIFF (Audio Interchange File Format)', () => {

Expand Down Expand Up @@ -159,4 +160,17 @@ describe('Parse AIFF (Audio Interchange File Format)', () => {
], 'common.comment');
});

it('Protect against CWE-835 with 0 chunk length', async () => {

const filePath = path.join(aiffSamplePath, 'CWE-835-01.aiff');

try {
await mm.parseFile(filePath);
expect.fail('Expected parseFile to throw UnexpectedFileContentError');
} catch (err) {
expect(err).to.be.instanceOf(UnexpectedFileContentError);
expect((err as Error).message).to.match(/COMMON CHUNK size should always be at least 22/);
}
});

});
36 changes: 26 additions & 10 deletions test/test-file-asf.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,32 @@ describe('Parse ASF', () => {

});

it('Avoid infinite loop CWE-835', async () => {
const filePath = path.join(asfFilePath, 'CWE-835.wma');

try {
await mm.parseFile(filePath);
expect.fail('Expected parseFile to throw AsfContentParseError');
} catch (err) {
expect(err).to.be.instanceOf(AsfContentParseError);
expect((err as Error).message).to.match(/Invalid ASF header object size/);
}
describe('security hardening', () => {

it('Avoid infinite loop CWE-835', async () => {
const filePath = path.join(asfFilePath, 'CWE-835.wma');

try {
await mm.parseFile(filePath);
expect.fail('Expected parseFile to throw AsfContentParseError');
} catch (err) {
expect(err).to.be.instanceOf(AsfContentParseError);
expect((err as Error).message).to.match(/Invalid ASF header object size/);
}
});

it('numberOfObjectHeaders=4294967295', async () => {
const filePath = path.join(asfFilePath, 'max-numberOfObjectHeaders.wma');

try {
await mm.parseFile(filePath);
expect.fail('Expected parseFile to throw AsfContentParseError');
} catch (err) {
expect(err).to.be.instanceOf(AsfContentParseError);
expect((err as Error).message).to.match(/Unrealistic number of ASF header objects/);
}
});

});

});
Loading