Gp3To5Importer: harden remaining four count-driven loops against DoS

[kaizenman]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

This is a follow-up to #2677 / PR #2670, which adds bounds checks to two count-driven loops in Gp3To5Importer (readBend, readTremoloBarEffect). Four additional count-driven loops in the same importer follow the same pattern and are theoretically vulnerable to the same DoS class — a crafted file declaring an oversized count field would skip past the PR #2670 checks and reach one of these:

Loop	Location
for i = 0; i < this._barCount; i++ (header bar count)	Gp3To5Importer.ts:336, :607
for i = 0; i < this._trackCount; i++ (header track count)	Gp3To5Importer.ts:452
for i = 0; i < beatCount; i++ (voice beat count)	Gp3To5Importer.ts:654

The original PR #2670 only fixes loops empirically observed to crash on naturally-corrupted files. A crafted file declaring barCount = 2_000_000_000 would still bypass it.

ByteBuffer.readByte() returns -1 silently at EOF rather than throwing, so past-EOF reads cannot terminate the runaway loop on the read side either.

Expected Behavior

Either:

1. Targeted approach: extend the _requireFits(data, count, bytesPerItem, fieldName) helper introduced in PR fix(importer): bound runaway loops on corrupt count fields in GP3-5 binary parser #2670 to the remaining four loops. Each loop has a known per-iteration byte cost, so the check is straightforward.
2. Holistic approach: change ByteBuffer.readByte() to throw EndOfReaderError at EOF instead of returning -1. Any past-EOF runaway loop terminates after a single iteration regardless of which count field drove it. Trade-off: contract change; existing callers that rely on the -1 sentinel must be audited.

Either yields the same end-state (no DoS via count-field corruption); they differ in surface area and risk.

Steps To Reproduce

Theoretical — no real-world fixture observed for these four loops (the same DoS class was found and fixed in readBend / readTremoloBarEffect via #2677; these four take the same input shape but were not encountered during reduction of the corrupted-file fixture in #2677). A crafted GP3/4/5 file with deliberately-inflated barCount / trackCount / beatCount header fields would trigger the same OOM-crash class as #2677.

Link to jsFiddle, CodePen, Project

No response

Version and Environment

alphaTab: 1.9.0 (commit f8ef24f, current HEAD of `develop`)

Platform

Web

Anything else?

This is the follow-up scope identified during PR #2670 review. Filing as a separate issue so the investigation isn't lost while the architectural decision is pending.

[Danielku15]

Good findings here too. I think it is similar to #2677 and we should harden our readers generally at key-points against DoS attacks in crafted files. Binary formats are more sensitive to such type of attacks, but I think we should generally harden the readers key-points against unrealistic data (loops/allocations etc.)

[Danielku15]

generally thanks a lot for re-creating the reports following the templates. they are all very good findings and your PRs already tackle some key aspects. I'll need some time to go through them in detail and consider further extensions around the topic. Very valuable findings.