From b44743fa0e4a7ae7a47bf3801ea1b0c784bdd9a5 Mon Sep 17 00:00:00 2001 From: kaizenman <15638776+kaizenman@users.noreply.github.com> Date: Sat, 25 Apr 2026 00:21:35 +0200 Subject: [PATCH 1/3] fix(importer): cap chord name read at field width in GP3-5 binary parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GpBinaryHelpers.gpReadStringByteLength interpreted the length-hint byte as the actual read count, advancing the stream by stringLength + 1 bytes. The field is fixed-width (length + 1 bytes) and the hint is just how many of those bytes are the active string — Guitar Pro itself, PyGuitarPro and TuxGuitar all read this way. When stringLength exceeds the field width (real-world GP5 chord names where Guitar Pro stored a hint of 32 in a 22-byte field) AlphaTab over-consumed the stream, mis-aligned the parse, and eventually attempted an unbounded readBend loop on garbage bytes — practical DoS against any page running AlphaTab. Read a fixed-width field (length + 1 bytes) and decode up to min(stringLength, length) bytes, matching the reference parsers. The buffer is also explicitly sized down before passing to IOHelper.toString since TextDecoder.decode(arr.buffer) ignores TypedArray byte offsets. The fixture test-data/guitarpro5/chord-name-overflow.gp5 has a chord diagram with a 32-byte name hint in a 22-byte field that previously triggered the runaway loop. Two regression tests: - end-to-end: file parses cleanly with expected track/measure counts - synthetic: gpReadStringByteLength stops at field width, leaving the stream pointer at length + 1 bytes regardless of the hint --- .../alphatab/src/importer/Gp3To5Importer.ts | 13 +++++---- .../guitarpro5/chord-name-overflow.gp5 | Bin 0 -> 19548 bytes .../test/importer/Gp5Importer.test.ts | 26 ++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) create mode 100755 packages/alphatab/test-data/guitarpro5/chord-name-overflow.gp5 diff --git a/packages/alphatab/src/importer/Gp3To5Importer.ts b/packages/alphatab/src/importer/Gp3To5Importer.ts index cc74873be..54d828eda 100644 --- a/packages/alphatab/src/importer/Gp3To5Importer.ts +++ b/packages/alphatab/src/importer/Gp3To5Importer.ts @@ -1571,12 +1571,15 @@ export class GpBinaryHelpers { * @returns */ public static gpReadStringByteLength(data: IReadable, length: number, encoding: string): string { + // Fixed-width string field: 1 length byte + `length` data bytes, decoded + // up to min(stringLength, length). Always consumes 1 + length bytes. const stringLength: number = data.readByte(); - const s: string = GpBinaryHelpers.gpReadString(data, stringLength, encoding); - if (stringLength < length) { - data.skip(length - stringLength); - } - return s; + const fieldBytes: Uint8Array = new Uint8Array(length); + data.read(fieldBytes, 0, length); + const effectiveLength: number = Math.max(0, Math.min(stringLength, length)); + const decoded: Uint8Array = new Uint8Array(effectiveLength); + decoded.set(fieldBytes.subarray(0, effectiveLength)); + return IOHelper.toString(decoded, encoding); } } diff --git a/packages/alphatab/test-data/guitarpro5/chord-name-overflow.gp5 b/packages/alphatab/test-data/guitarpro5/chord-name-overflow.gp5 new file mode 100755 index 0000000000000000000000000000000000000000..8de558027e98606d54b9a84e8dca7fd772f62cc8 GIT binary patch literal 19548 zcmeHOUvC@75#K#h7OlvRqNauGw#K$FgA}M!)^%Y4?E@7#kyOOBD#-}aH`=;bL}^kW zsX+P|ee2gL`qJlof_{p=y7Qa4-5uU76Ma;JlXkTcYAYdzr98y57!3+w~LX9+s!BW_^bSY)U`dF=Hp>++8+&j0~c5C<iYB1@nlL{@jdq_ z|FU;Hm}1wz8M^mJ!+aw4Z|`>OLc^@th0O4dXj|05(CiAe*afUB`hcy=gT?A5Eo>AE zFXD2#2=kkIQ1sGz+Zx9>9o#j?i}>6Mn( zO4snkM#_>lkhJF;tA*YuI&hO*-K7Br#-hO#5uEgSP!)!?oI58B+tMFG%UBX!aYY4K zVs@e{0*IkXs|F$76g=NRbazJGG)?PhdLUDdkJ^x(cX`v zgb!J;$%0WaJ}9Yz1sr%yE;HLQdroMtae_A$N&M@H}*aOGk{2a4e4W<@F=$cO+gf`!oAqUi(nF@ zr!@%a*dQH&IP1DeYL}ReDD*6>VTm%YmS8s_iDpK0bC*FtqsJL)W%4y1ShMsbfheO6 zribA`gBlLuE5l*QK#olx1WAqrBEWe-ARtHd74&3xXCKMb7^xDbfF(bo0ii5_<#>^; zd9aF2QaF>xqAo)e^^jT70D)ENQEM2s{W-yA))mRAK95h)y$IpKwLRUi6efi|s%YK* z6A+Y9036F6HdC|`5*J3GIc!?Z1(6RNg4WPYI2t6#O-`RMZIOCQ-wi+iav^IG2gtf@!O2HI z4k-BQ5B$mxKA6K%PNsW!6Ew--DrdLH7FcNSa! zBVHgGB^}|7gid@HWx5l7-BGTvlmF1ssp0M@x9rI7L9{%_GKi+%9edq3#=bz+);x?p*)5of6jMG`t>Z&^LU2C5T9YE!i5QwfW+*n{S6 zw5qIwI7Dj;!AtR^^olE#hop=G*lLH8-CaTC>9W%!58w?amhNoGHn?I_+M^^4)PG2? zBw2`E*|0>qx=_Ss7EXIgK+#O=LQ%?eRQQV-)CI~0-NNgY=sCV-3PA|Mb)kq<(Pf)8 z;=xKv--#M7pm87PmBxjaA7r7Zv^$C<;1ry~a@mUexP{5aZgDt|^)6OCCMk(Z27bab z>Bcp(1yv)n(1MhQPN?d(AnzHtAMK|XSri;uCAKz;tbh2vN}E7?lO zQo9@HGcp||3TfS2stb_QrTav2)&cNarAdGN5_%`TiL?>x$DG z{o2Z>Mc4(&0w~BQT6M*Vq*H&IBo?CLX!J!20%APmvt&cTkTALaG)W&eFZt>F;)ctVu6c0%O@uL+G>Kw~QQ{MM1KIdaP8sS*X zZ&la>>DFaT#sBBG)-swLshAG}RA971g?iy14#>tn^HjvcIscajkS9RQQo$4^LClp! znu-utZMP6{1vixG|8vAcH!5bGxv;9hb71Utz%5-0OBM5(grX$DFqFhS=E_1B@OfTR zm@CU5n8m_0fKMtN=OFYA;geLua~0n=RS6BmMKY_Ls%2uv_{2!6_(z;IwpK_jIINOC RtYq)R<{}JMEsxFE`aefV4iEqU literal 0 HcmV?d00001 diff --git a/packages/alphatab/test/importer/Gp5Importer.test.ts b/packages/alphatab/test/importer/Gp5Importer.test.ts index d114b0d88..189791661 100644 --- a/packages/alphatab/test/importer/Gp5Importer.test.ts +++ b/packages/alphatab/test/importer/Gp5Importer.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest'; import { Settings } from '@coderline/alphatab/Settings'; +import { GpBinaryHelpers } from '@coderline/alphatab/importer/Gp3To5Importer'; +import { ByteBuffer } from '@coderline/alphatab/io/ByteBuffer'; import { type Beat, BeatBeamingMode } from '@coderline/alphatab/model/Beat'; import { Direction } from '@coderline/alphatab/model/Direction'; import { Ottavia } from '@coderline/alphatab/model/Ottavia'; @@ -569,4 +571,28 @@ describe('Gp5ImporterTest', () => { } } }); + + it('chord-name-overflow', async () => { + // GP5 file with a chord name length byte that exceeds the 21-byte field + // (length=32). Pre-fix, gpReadStringByteLength consumed the full 32 bytes, + // mis-aligning the stream and triggering an unbounded readBend loop. + const score = ( + await GpImporterTestHelper.prepareImporterWithFile('guitarpro5/chord-name-overflow.gp5') + ).readScore(); + expect(score.tracks.length).to.equal(1); + expect(score.masterBars.length).to.equal(193); + }); + + it('gpReadStringByteLength caps consumption at field width', () => { + const sentinelByte = 0xca; + const fieldSize = 21; + const overlongHint = 32; + const buffer = ByteBuffer.fromBuffer( + new Uint8Array([overlongHint, ...new Array(fieldSize).fill(0x41), sentinelByte]) + ); + const result = GpBinaryHelpers.gpReadStringByteLength(buffer, fieldSize, 'utf-8'); + expect(result).to.equal('A'.repeat(fieldSize)); + expect(buffer.position).to.equal(1 + fieldSize); + expect(buffer.readByte()).to.equal(sentinelByte); + }); }); From e7c8c6b0e1e5d83335a445c36c912df22021dd1c Mon Sep 17 00:00:00 2001 From: Danielku15 Date: Fri, 15 May 2026 16:14:13 +0200 Subject: [PATCH 2/3] perf: avoid additional array allocation --- packages/alphatab/src/importer/Gp3To5Importer.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/alphatab/src/importer/Gp3To5Importer.ts b/packages/alphatab/src/importer/Gp3To5Importer.ts index 54d828eda..9d564d3f2 100644 --- a/packages/alphatab/src/importer/Gp3To5Importer.ts +++ b/packages/alphatab/src/importer/Gp3To5Importer.ts @@ -57,7 +57,7 @@ export class Gp3To5Importer extends ScoreImporter { // NOTE: General Midi only defines percussion instruments from 35-81 // Guitar Pro 5 allowed GS extensions (27-34 and 82-87) - // GP7-8 do not have all these definitions anymore, this lookup ensures some fallback + // GP7-8 do not have all these definitions anymore, this lookup ensures some fallback // (even if they are not correct) // we can support this properly in future when we allow custom alphaTex articulation definitions // then we don't need to rely on GP specifics anymore but handle things on export/import @@ -1573,13 +1573,11 @@ export class GpBinaryHelpers { public static gpReadStringByteLength(data: IReadable, length: number, encoding: string): string { // Fixed-width string field: 1 length byte + `length` data bytes, decoded // up to min(stringLength, length). Always consumes 1 + length bytes. - const stringLength: number = data.readByte(); - const fieldBytes: Uint8Array = new Uint8Array(length); + const stringLength = data.readByte(); + const fieldBytes = new Uint8Array(length); data.read(fieldBytes, 0, length); - const effectiveLength: number = Math.max(0, Math.min(stringLength, length)); - const decoded: Uint8Array = new Uint8Array(effectiveLength); - decoded.set(fieldBytes.subarray(0, effectiveLength)); - return IOHelper.toString(decoded, encoding); + const effectiveLength = Math.min(stringLength, length); + return IOHelper.toString(fieldBytes.subarray(0, effectiveLength), encoding); } } From 5ab9da5743d3d18f1d86a6440d8a1fadba6942e4 Mon Sep 17 00:00:00 2001 From: Danielku15 Date: Fri, 15 May 2026 16:19:23 +0200 Subject: [PATCH 3/3] perf: avoid buffer copy for gp string --- packages/alphatab/src/io/IOHelper.ts | 2 +- .../test/importer/Gp5Importer.test.ts | 43 +++++++++---------- .../AlphaTab/Core/EcmaScript/TextDecoder.cs | 5 +++ .../src/main/java/alphaTab/core/Globals.kt | 6 --- .../alphaTab/core/ecmaScript/TextDecoder.kt | 9 +++- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/alphatab/src/io/IOHelper.ts b/packages/alphatab/src/io/IOHelper.ts index db8151068..2d3814ed8 100644 --- a/packages/alphatab/src/io/IOHelper.ts +++ b/packages/alphatab/src/io/IOHelper.ts @@ -155,7 +155,7 @@ export class IOHelper { encoding = 'utf-8'; } const decoder: TextDecoder = new TextDecoder(encoding); - return decoder.decode(data.buffer as ArrayBuffer); + return decoder.decode(data); } private static _detectEncoding(data: Uint8Array): string | null { diff --git a/packages/alphatab/test/importer/Gp5Importer.test.ts b/packages/alphatab/test/importer/Gp5Importer.test.ts index 189791661..d954477d7 100644 --- a/packages/alphatab/test/importer/Gp5Importer.test.ts +++ b/packages/alphatab/test/importer/Gp5Importer.test.ts @@ -409,9 +409,7 @@ describe('Gp5ImporterTest', () => { expect(score.tracks[0].staves[0].bars[1].voices[0].beats[3].preferredBeamDirection).not.toBeTruthy(); // break - expect(score.tracks[0].staves[0].bars[2].voices[0].beats[0].beamingMode).toBe( - BeatBeamingMode.ForceSplitToNext - ); + expect(score.tracks[0].staves[0].bars[2].voices[0].beats[0].beamingMode).toBe(BeatBeamingMode.ForceSplitToNext); expect(score.tracks[0].staves[0].bars[2].voices[0].beats[0].invertBeamDirection).toBe(false); expect(score.tracks[0].staves[0].bars[2].voices[0].beats[0].preferredBeamDirection).not.toBeTruthy(); @@ -421,9 +419,7 @@ describe('Gp5ImporterTest', () => { expect(score.tracks[0].staves[0].bars[2].voices[0].beats[1].invertBeamDirection).toBe(false); expect(score.tracks[0].staves[0].bars[2].voices[0].beats[1].preferredBeamDirection).not.toBeTruthy(); - expect(score.tracks[0].staves[0].bars[2].voices[0].beats[2].beamingMode).toBe( - BeatBeamingMode.ForceSplitToNext - ); + expect(score.tracks[0].staves[0].bars[2].voices[0].beats[2].beamingMode).toBe(BeatBeamingMode.ForceSplitToNext); expect(score.tracks[0].staves[0].bars[2].voices[0].beats[2].invertBeamDirection).toBe(false); expect(score.tracks[0].staves[0].bars[2].voices[0].beats[2].preferredBeamDirection).not.toBeTruthy(); @@ -449,9 +445,7 @@ describe('Gp5ImporterTest', () => { // invert to down expect(score.tracks[0].staves[0].bars[4].voices[0].beats[0].beamingMode).toBe(BeatBeamingMode.Auto); expect(score.tracks[0].staves[0].bars[4].voices[0].beats[0].invertBeamDirection).toBe(false); - expect(score.tracks[0].staves[0].bars[4].voices[0].beats[0].preferredBeamDirection).toBe( - BeamDirection.Down - ); + expect(score.tracks[0].staves[0].bars[4].voices[0].beats[0].preferredBeamDirection).toBe(BeamDirection.Down); // invert to up expect(score.tracks[0].staves[0].bars[5].voices[0].beats[0].beamingMode).toBe(BeatBeamingMode.Auto); @@ -525,18 +519,14 @@ describe('Gp5ImporterTest', () => { expect(score.style!.headerAndFooter.has(ScoreSubElement.Transcriber)).toBe(false); expect(score.style!.headerAndFooter.has(ScoreSubElement.Copyright)).toBe(true); - expect(score.style!.headerAndFooter.get(ScoreSubElement.Copyright)!.template).toBe( - 'Copyright: %COPYRIGHT%' - ); + expect(score.style!.headerAndFooter.get(ScoreSubElement.Copyright)!.template).toBe('Copyright: %COPYRIGHT%'); expect(score.style!.headerAndFooter.get(ScoreSubElement.Copyright)!.isVisible).toBe(true); expect(score.style!.headerAndFooter.get(ScoreSubElement.Copyright)!.textAlign).toBe(TextAlign.Center); expect(score.style!.headerAndFooter.has(ScoreSubElement.CopyrightSecondLine)).toBe(true); expect(score.style!.headerAndFooter.get(ScoreSubElement.CopyrightSecondLine)!.template).toBe('Copyright2'); expect(score.style!.headerAndFooter.get(ScoreSubElement.CopyrightSecondLine)!.isVisible).toBe(true); - expect(score.style!.headerAndFooter.get(ScoreSubElement.CopyrightSecondLine)!.textAlign).toBe( - TextAlign.Center - ); + expect(score.style!.headerAndFooter.get(ScoreSubElement.CopyrightSecondLine)!.textAlign).toBe(TextAlign.Center); }); it('bank', async () => { @@ -579,20 +569,27 @@ describe('Gp5ImporterTest', () => { const score = ( await GpImporterTestHelper.prepareImporterWithFile('guitarpro5/chord-name-overflow.gp5') ).readScore(); - expect(score.tracks.length).to.equal(1); - expect(score.masterBars.length).to.equal(193); + expect(score.tracks.length).toBe(1); + expect(score.masterBars.length).toBe(193); }); it('gpReadStringByteLength caps consumption at field width', () => { const sentinelByte = 0xca; const fieldSize = 21; const overlongHint = 32; - const buffer = ByteBuffer.fromBuffer( - new Uint8Array([overlongHint, ...new Array(fieldSize).fill(0x41), sentinelByte]) - ); + + const raw = new Uint8Array(fieldSize + 2); + raw[0] = overlongHint; + for(let i = 0; i < fieldSize; i++) { + raw[i + 1] = 0x41; + } + raw[fieldSize + 1] = sentinelByte; + + const buffer = ByteBuffer.fromBuffer(raw); + const result = GpBinaryHelpers.gpReadStringByteLength(buffer, fieldSize, 'utf-8'); - expect(result).to.equal('A'.repeat(fieldSize)); - expect(buffer.position).to.equal(1 + fieldSize); - expect(buffer.readByte()).to.equal(sentinelByte); + expect(result).toBe('A'.repeat(fieldSize)); + expect(buffer.position).toBe(1 + fieldSize); + expect(buffer.readByte()).toBe(sentinelByte); }); }); diff --git a/packages/csharp/src/AlphaTab/Core/EcmaScript/TextDecoder.cs b/packages/csharp/src/AlphaTab/Core/EcmaScript/TextDecoder.cs index 6f1ca5a56..c84b8fd37 100644 --- a/packages/csharp/src/AlphaTab/Core/EcmaScript/TextDecoder.cs +++ b/packages/csharp/src/AlphaTab/Core/EcmaScript/TextDecoder.cs @@ -15,4 +15,9 @@ public string Decode(ArrayBuffer data) { return _encoding.GetString(data.Raw, 0, (int)data.ByteLength); } + + public string Decode(Uint8Array data) + { + return _encoding.GetString(data.Buffer.Raw, (int)data.ByteOffset, (int)data.Length); + } } diff --git a/packages/kotlin/src/android/src/main/java/alphaTab/core/Globals.kt b/packages/kotlin/src/android/src/main/java/alphaTab/core/Globals.kt index 798dee2ec..679418d08 100644 --- a/packages/kotlin/src/android/src/main/java/alphaTab/core/Globals.kt +++ b/packages/kotlin/src/android/src/main/java/alphaTab/core/Globals.kt @@ -40,12 +40,6 @@ internal inline fun UByteArray.decodeToDoubleArray(): DoubleArray { return da } -@ExperimentalUnsignedTypes -internal inline fun UByteArray.decodeToString(encoding: String): String { - return String(this.toByteArray(), 0, this.size, Charset.forName(encoding)) -} - - internal inline fun > List.sort() { this.sort { a, b -> a.compareTo(b).toDouble() diff --git a/packages/kotlin/src/android/src/main/java/alphaTab/core/ecmaScript/TextDecoder.kt b/packages/kotlin/src/android/src/main/java/alphaTab/core/ecmaScript/TextDecoder.kt index 61b58483d..59a95ecb7 100644 --- a/packages/kotlin/src/android/src/main/java/alphaTab/core/ecmaScript/TextDecoder.kt +++ b/packages/kotlin/src/android/src/main/java/alphaTab/core/ecmaScript/TextDecoder.kt @@ -1,13 +1,18 @@ package alphaTab.core.ecmaScript -import alphaTab.core.decodeToString import alphaTab.core.ecmaScript.ArrayBuffer +import java.nio.charset.Charset internal class TextDecoder(encoding:String) { private val _encoding:String = encoding @ExperimentalUnsignedTypes public fun decode(buffer: ArrayBuffer): String { - return buffer.decodeToString(_encoding) + return String(buffer.toByteArray(), 0, buffer.size, Charset.forName(_encoding)) + } + + @ExperimentalUnsignedTypes + public fun decode(buffer: Uint8Array): String { + return String(buffer.buffer.toByteArray(), buffer.byteOffset.toInt(), buffer.length.toInt(), Charset.forName(_encoding)) } }