From 1c3637de4f595e8d159d814701bf4083f1fcdacb 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] 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 4ef957e89..0de09721b 100644 --- a/packages/alphatab/test/importer/Gp5Importer.test.ts +++ b/packages/alphatab/test/importer/Gp5Importer.test.ts @@ -1,4 +1,6 @@ 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); + }); });