fix(importer): reject malformed GPX containers instead of looping forever

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()) {

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) {

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') ||

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();