From 1f0c3f3d540daea4b53043ce310b4806f59d0cea Mon Sep 17 00:00:00 2001 From: e-c-hansen Date: Mon, 31 Aug 2026 19:40:26 -0400 Subject: [PATCH] ReadXM: fix out-of-bounds reads from unvalidated dwMemPos In CSoundFile::ReadXM the instrument-sample loop advances dwMemPos by the attacker-controlled samplesize[] values with no overflow guard, so on exit dwMemPos can be far past dwMemLength (e.g. 0xFFFFFFF8). The tail parsers then trust it with overflow-prone checks: - "TEXT" block: (dwMemPos + 8 < dwMemLength) wraps to 0 < dwMemLength and reads *(DWORD*)(lpStream + dwMemPos) ~4 GB out of bounds - "MIDI" block: same shape - mix plugins: LoadMixPlugins(lpStream + dwMemPos, ...) A crafted .xm file triggers a SIGSEGV out-of-bounds read (confirmed with AddressSanitizer and on stock builds). Guarding a single site just moves the crash to the next, so clamp dwMemPos to dwMemLength once, after the sample loop. Each tail parser already compares dwMemPos against dwMemLength and only misbehaved when dwMemPos was allowed to exceed it. Tested: crashing input now parses cleanly under ASan; valid .xm/.mod/.s3m/.it files are unaffected. --- src/load_xm.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/load_xm.cpp b/src/load_xm.cpp index 4d5a493f..fe730f78 100644 --- a/src/load_xm.cpp +++ b/src/load_xm.cpp @@ -518,6 +518,11 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) if (dwMemPos >= dwMemLength) break; } } + // SECURITY: sample sizes are attacker-controlled; the loop above can + // leave dwMemPos past the end of the buffer (or wrapped). Every tail + // parser below (TEXT, MIDI, mix plugins) trusts dwMemPos, so clamp once + // here rather than guarding each site against integer overflow. + if (dwMemPos > dwMemLength) dwMemPos = dwMemLength; // Read song comments: "TEXT" if ((dwMemPos + 8 < dwMemLength) && (bswapLE32(*((DWORD *)(lpStream+dwMemPos))) == 0x74786574)) {