Harden ASF parser against infinite loop and malformed headers
- Prevent parser rewind by disallowing negative skip lengths in tokenizer - Validate ASF object sizes to ensure minimum header length - Add sanity limit (10k) for numberOfHeaderObjects to avoid excessive iteration
This commit is contained in:
+1
-1
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
};
|
||||
}
|
||||
};
|
||||
@@ -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;
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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) {
|
||||
|
||||
|
||||
Binary file not shown.
Binary file not shown.
Binary file not shown.
+15
-1
@@ -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)', () => {
|
||||
|
||||
@@ -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/);
|
||||
}
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -154,6 +154,8 @@ describe('Parse ASF', () => {
|
||||
|
||||
});
|
||||
|
||||
describe('security hardening', () => {
|
||||
|
||||
it('Avoid infinite loop CWE-835', async () => {
|
||||
const filePath = path.join(asfFilePath, 'CWE-835.wma');
|
||||
|
||||
@@ -166,4 +168,18 @@ describe('Parse ASF', () => {
|
||||
}
|
||||
});
|
||||
|
||||
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/);
|
||||
}
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user