From e3b649849c571c13ae3ccdf9471b303f69869ff7 Mon Sep 17 00:00:00 2001 From: Danielku15 Date: Thu, 29 Oct 2026 18:22:17 +0100 Subject: [PATCH] fix(importer): reject malformed GPX containers instead of looping forever A file entry whose data sectors lie before the entry made the sector scan revisit the same entry endlessly while allocating memory. The scan now only moves forward, sector pointers and file sizes are validated against the container, the decompressed size respects maxDecodingBufferSize and a score.gpif without root element is rejected as unsupported. Fixes #2967 --- packages/alphatab/src/importer/GpifParser.ts | 5 +- .../alphatab/src/importer/GpxFileSystem.ts | 58 ++++++++----- packages/alphatab/src/importer/GpxImporter.ts | 2 +- .../test/importer/GpxImporter.test.ts | 85 ++++++++++++++++++- 4 files changed, 125 insertions(+), 25 deletions(-) diff --git a/packages/alphatab/src/importer/GpifParser.ts b/packages/alphatab/src/importer/GpifParser.ts index 5435f95133..cb4c9ca759 100644 --- a/packages/alphatab/src/importer/GpifParser.ts +++ b/packages/alphatab/src/importer/GpifParser.ts @@ -208,14 +208,11 @@ export class GpifParser { private _parseDom(dom: XmlDocument): void { const root: XmlNode | null = dom.firstElement; - if (!root) { - return; - } // the XML uses IDs for referring elements within the // Therefore we do the parsing in 2 steps: // - at first we read all model elements and store them by ID in a lookup table // - after that we need to join up the information. - if (root.localName === 'GPIF') { + if (root && root.localName === 'GPIF') { this.score = new Score(); // parse all children for (const n of root.childElements()) { diff --git a/packages/alphatab/src/importer/GpxFileSystem.ts b/packages/alphatab/src/importer/GpxFileSystem.ts index 03162a4939..7a44cdf91d 100644 --- a/packages/alphatab/src/importer/GpxFileSystem.ts +++ b/packages/alphatab/src/importer/GpxFileSystem.ts @@ -1,7 +1,7 @@ import { UnsupportedFormatError } from '@coderline/alphatab/importer/UnsupportedFormatError'; import { BitReader } from '@coderline/alphatab/io/BitReader'; import { ByteBuffer } from '@coderline/alphatab/io/ByteBuffer'; -import { EndOfReaderError, type IReadable } from '@coderline/alphatab/io/IReadable'; +import { EndOfReaderError, type IReadable, OverflowError } from '@coderline/alphatab/io/IReadable'; /** * this public class represents a file within the GpxFileSystem @@ -35,10 +35,14 @@ export class GpxFileSystem { */ public files: GpxFile[] = []; + private _maxDecodingBufferSize: number; + /** * Creates a new GpxFileSystem instance + * @param maxDecodingBufferSize the maximum allowed size of a decompressed BCFZ container. */ - public constructor() { + public constructor(maxDecodingBufferSize: number) { + this._maxDecodingBufferSize = maxDecodingBufferSize; this.files = []; this.fileFilter = _ => { return true; @@ -76,6 +80,9 @@ export class GpxFileSystem { const uncompressed: ByteBuffer = ByteBuffer.empty(); let buffer: Uint8Array; const expectedLength: number = this._getInteger(src.readBytes(4), 0); + if (expectedLength > this._maxDecodingBufferSize) { + throw new OverflowError('GPX container exceeds the configured maxDecodingBufferSize'); + } try { // as long we reach our expected length we try to decompress, a EOF might occure. while (uncompressed.length < expectedLength) { @@ -107,8 +114,11 @@ export class GpxFileSystem { throw e; } } - buffer = uncompressed.getBuffer(); const resultOffset: number = skipHeader ? 4 : 0; + if (uncompressed.length < resultOffset) { + throw new UnsupportedFormatError('GPX container is truncated'); + } + buffer = uncompressed.getBuffer(); const resultSize: number = uncompressed.length - resultOffset; const result: Uint8Array = new Uint8Array(resultSize); const count: number = resultSize; @@ -159,35 +169,45 @@ export class GpxFileSystem { // 0x90 | ? | 4byte | Unknown // 0x94 | int[] | n*4byte | Indices of the sector containing the data (end is marked with 0) // The sectors marked at 0x94 are absolutely positioned ( 1*0x1000 is sector 1, 2*0x1000 is sector 2,...) + const dataPointerOffset: number = offset + 0x94; + if (dataPointerOffset + 4 > data.length) { + throw new UnsupportedFormatError('GPX file entry is truncated'); + } const file: GpxFile = new GpxFile(); file.fileName = this._getString(data, offset + 0x04, 127); file.fileSize = this._getInteger(data, offset + 0x8c); + if (file.fileSize < 0 || file.fileSize > data.length) { + throw new UnsupportedFormatError(`GPX file entry '${file.fileName}' has an invalid size`); + } // store file if needed const storeFile: boolean = !this.fileFilter || this.fileFilter(file.fileName); if (storeFile) { this.files.push(file); } - // we need to iterate the blocks because we need to move after the last datasector - const dataPointerOffset: number = offset + 0x94; - let sector: number = 0; - // this var is storing the sector index let sectorCount: number = 0; - // we're keeping count so we can calculate the offset of the array item - // as long we have data blocks we need to iterate them, const fileData: ByteBuffer | null = storeFile ? ByteBuffer.withCapacity(file.fileSize) : null; while (true) { - sector = this._getInteger(data, dataPointerOffset + 4 * sectorCount++); - if (sector !== 0) { - // the next file entry starts after the last data sector so we - // move the offset along - offset = sector * sectorSize; - // write data only if needed - if (storeFile) { - fileData!.write(data, offset, sectorSize); - } - } else { + const pointerOffset: number = dataPointerOffset + 4 * sectorCount++; + if (pointerOffset + 4 > data.length) { + throw new UnsupportedFormatError( + `GPX file entry '${file.fileName}' has an unterminated sector list` + ); + } + const sector: number = this._getInteger(data, pointerOffset); + if (sector === 0) { break; } + const sectorOffset: number = sector * sectorSize; + if (sector < 0 || sectorOffset >= data.length) { + throw new UnsupportedFormatError( + `GPX file entry '${file.fileName}' points to sector ${sector} outside of the container` + ); + } + // the next entry follows the furthest data sector + offset = Math.max(offset, sectorOffset); + if (storeFile && fileData!.length < file.fileSize) { + fileData!.write(data, sectorOffset, sectorSize); + } } if (storeFile) { diff --git a/packages/alphatab/src/importer/GpxImporter.ts b/packages/alphatab/src/importer/GpxImporter.ts index 421616ee46..9e1eec270a 100644 --- a/packages/alphatab/src/importer/GpxImporter.ts +++ b/packages/alphatab/src/importer/GpxImporter.ts @@ -22,7 +22,7 @@ export class GpxImporter extends ScoreImporter { // at first we need to load the binary file system // from the GPX container Logger.debug(this.name, 'Loading GPX filesystem'); - const fileSystem: GpxFileSystem = new GpxFileSystem(); + const fileSystem: GpxFileSystem = new GpxFileSystem(this.settings.importer.maxDecodingBufferSize); fileSystem.fileFilter = s => { return ( s.endsWith('score.gpif') || diff --git a/packages/alphatab/test/importer/GpxImporter.test.ts b/packages/alphatab/test/importer/GpxImporter.test.ts index 0e52065fd8..a90c6505e1 100644 --- a/packages/alphatab/test/importer/GpxImporter.test.ts +++ b/packages/alphatab/test/importer/GpxImporter.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest'; import { type GpxFile, GpxFileSystem } from '@coderline/alphatab/importer/GpxFileSystem'; import { GpxImporter } from '@coderline/alphatab/importer/GpxImporter'; +import { UnsupportedFormatError } from '@coderline/alphatab/importer/UnsupportedFormatError'; +import { ScoreLoader } from '@coderline/alphatab/importer/ScoreLoader'; import { ByteBuffer } from '@coderline/alphatab/io/ByteBuffer'; +import { IOHelper } from '@coderline/alphatab/io/IOHelper'; +import { OverflowError } from '@coderline/alphatab/io/IReadable'; import type { Score } from '@coderline/alphatab/model/Score'; import { Settings } from '@coderline/alphatab/Settings'; import { Logger } from '@coderline/alphatab/Logger'; @@ -21,7 +25,7 @@ describe('GpxImporterTest', () => { it('file-system-compressed', async () => { const data = await TestPlatform.loadFile('test-data/guitarpro6/file-system-compressed.gpx'); - const fileSystem: GpxFileSystem = new GpxFileSystem(); + const fileSystem: GpxFileSystem = new GpxFileSystem(new Settings().importer.maxDecodingBufferSize); fileSystem.load(ByteBuffer.fromBuffer(data)); const names: string[] = [ 'score.gpif', @@ -39,6 +43,85 @@ describe('GpxImporterTest', () => { } }); + /** + * Builds a BCFS container with one score.gpif entry. Sector indices are relative to the data after the header. + */ + function buildBcfsContainer( + sectorCount: number, + entrySector: number, + fileSize: number, + sectorPointers: number[] + ): Uint8Array { + const buffer = ByteBuffer.fromBuffer(new Uint8Array(4 + sectorCount * 0x1000)); + buffer.write(IOHelper.stringToBytes('BCFS'), 0, 4); + const entry = 4 + entrySector * 0x1000; + buffer.position = entry; + IOHelper.writeInt32LE(buffer, 2); + buffer.write(IOHelper.stringToBytes('score.gpif'), 0, 10); + buffer.position = entry + 0x8c; + IOHelper.writeInt32LE(buffer, fileSize); + buffer.position = entry + 0x94; + for (const pointer of sectorPointers) { + IOHelper.writeInt32LE(buffer, pointer); + } + return buffer.toArray(); + } + + function loadFileSystem(data: Uint8Array): GpxFileSystem { + const fileSystem: GpxFileSystem = new GpxFileSystem(new Settings().importer.maxDecodingBufferSize); + fileSystem.load(ByteBuffer.fromBuffer(data)); + return fileSystem; + } + + it('file-system-sector-before-entry', () => { + const fileSystem = loadFileSystem(buildBcfsContainer(3, 2, 1, [1])); + expect(fileSystem.files.length).toBe(1); + expect(fileSystem.files[0].fileName).toBe('score.gpif'); + }); + + it('file-system-negative-sector', () => { + expect(() => loadFileSystem(buildBcfsContainer(3, 2, 1, [-1]))).toThrow(UnsupportedFormatError); + }); + + it('file-system-sector-past-end', () => { + expect(() => loadFileSystem(buildBcfsContainer(3, 2, 1, [3]))).toThrow(UnsupportedFormatError); + }); + + it('file-system-unterminated-sector-list', () => { + const pointers: number[] = []; + for (let i = 0; i < (0x1000 - 0x94) / 4; i++) { + pointers.push(1); + } + expect(() => loadFileSystem(buildBcfsContainer(3, 2, 1, pointers))).toThrow(UnsupportedFormatError); + }); + + it('file-system-negative-file-size', () => { + expect(() => loadFileSystem(buildBcfsContainer(3, 2, -1, [1]))).toThrow(UnsupportedFormatError); + }); + + it('file-system-file-size-exceeds-container', () => { + expect(() => loadFileSystem(buildBcfsContainer(3, 2, 0x7fffffff, [1]))).toThrow(UnsupportedFormatError); + }); + + it('file-system-decompressed-size-exceeds-limit', () => { + const buffer = ByteBuffer.empty(); + buffer.write(IOHelper.stringToBytes('BCFZ'), 0, 4); + IOHelper.writeInt32LE(buffer, 0x7fffffff); + expect(() => loadFileSystem(buffer.toArray())).toThrow(OverflowError); + }); + + it('file-system-decompressed-size-below-header', () => { + const buffer = ByteBuffer.empty(); + buffer.write(IOHelper.stringToBytes('BCFZ'), 0, 4); + IOHelper.writeInt32LE(buffer, 2); + expect(() => loadFileSystem(buffer.toArray())).toThrow(UnsupportedFormatError); + }); + + it('score-loader-malformed-gpx-terminates', () => { + const data = buildBcfsContainer(3, 2, 1, [1]); + expect(() => ScoreLoader.loadScoreFromBytes(data)).toThrow(UnsupportedFormatError); + }); + it('score-info', async () => { const reader = await prepareImporterWithFile('guitarpro6/score-info.gpx'); const score: Score = reader.readScore();