diff --git a/lib/ParseError.ts b/lib/ParseError.ts index 0a1430ee..30e39374 100644 --- a/lib/ParseError.ts +++ b/lib/ParseError.ts @@ -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) { diff --git a/lib/asf/AsfObject.ts b/lib/asf/AsfObject.ts index 7651a540..180e3a07 100644 --- a/lib/asf/AsfObject.ts +++ b/lib/asf/AsfObject.ts @@ -77,11 +77,12 @@ export const TopLevelHeaderObjectToken: IGetToken { + 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 = { 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; } }; diff --git a/lib/asf/AsfParser.ts b/lib/asf/AsfParser.ts index 0eb97c35..845429ab 100644 --- a/lib/asf/AsfParser.ts +++ b/lib/asf/AsfParser.ts @@ -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.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.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) { diff --git a/test/samples/aiff/CWE-835-01.aiff b/test/samples/aiff/CWE-835-01.aiff new file mode 100644 index 00000000..b5573b06 Binary files /dev/null and b/test/samples/aiff/CWE-835-01.aiff differ diff --git a/test/samples/asf/CWE-835-03.wma b/test/samples/asf/CWE-835-03.wma new file mode 100644 index 00000000..dd31a8c7 Binary files /dev/null and b/test/samples/asf/CWE-835-03.wma differ diff --git a/test/samples/asf/max-numberOfObjectHeaders.wma b/test/samples/asf/max-numberOfObjectHeaders.wma new file mode 100644 index 00000000..760da1dc Binary files /dev/null and b/test/samples/asf/max-numberOfObjectHeaders.wma differ diff --git a/test/test-file-aiff.ts b/test/test-file-aiff.ts index 1dd67ebc..891070cf 100644 --- a/test/test-file-aiff.ts +++ b/test/test-file-aiff.ts @@ -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/); + } + }); + }); diff --git a/test/test-file-asf.ts b/test/test-file-asf.ts index 05fd42ec..b9838371 100644 --- a/test/test-file-asf.ts +++ b/test/test-file-asf.ts @@ -154,16 +154,32 @@ describe('Parse ASF', () => { }); - it('Avoid infinite loop CWE-835', async () => { - const filePath = path.join(asfFilePath, 'CWE-835.wma'); + 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/); + } + }); - 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/); - } }); });