[PATCH] frontend: handle mpg123 length and getstate errors in decode

"Maya R. Odinezenko via Lame-dev" <[email protected]> Fri, 17 Jul 2026 15:18:00 -0400
Newsgroups gmane.comp.audio.mp3.lame
Message-ID <CAFCgL3n_kRiULFacq9hMLvoERZc7pGxXb4R+48kfqdT3JqwqpA@mail.gmail.com>
Hello,

In the libmpg123 decode path, lame123_decode_initfile()
(frontend/get_audio.c) does not check the error returns from
mpg123_framelength() and mpg123_length(). Both return MPG123_ERR (-1)
when no length is known, but the guards (len <= (unsigned long)-1 and
len <= UINT_MAX/2) admit that -1 before it is stored into the
destination fields, and the first cannot reject anything at all where
unsigned long is 64-bit. With a piped, Info-tag-less MP3 I saw nsamp
become 2^64-1, which also masks the MAX_U_32_NUM "unknown" sentinel
and disables the file-size fallback. The two mpg123_getstate() calls
just below are likewise unchecked and read val uninitialized if the
query fails.

The patch stores 0 (unknown) for an unusable frame count, MAX_U_32_NUM
(unknown) for a failed length query so the file-size fallback
re-engages, keeps rejecting counts above INT_MAX as before, and on a
failed getstate() uses the documented "unknown" value (-1) instead of
reading val uninitialized.

Tested on Apple Silicon (LP64): the piped decode now stores the
sentinel instead of 2^64-1; seekable and LAME-tagged inputs are
unchanged (delay/padding read identically); a forged oversized-Xing
file is still rejected; decoded and encoded outputs are byte-identical.

The patch is against current trunk (r6597).

Maya

-- >8 --
From: Maya <[email protected]>
Date: Fri, 17 Jul 2026 14:30:00 -0400
Subject: [PATCH] frontend: handle mpg123 length and getstate errors in decode
 init

mpg123_framelength() and mpg123_length() return MPG123_ERR (-1) when
no length guess is possible, but both guards admitted negative values
before conversion into the destination fields, and the framelength
guard (len <= (unsigned long)-1) cannot reject anything where unsigned
long is 64-bit. Reproduced with a piped, Info-tag-less MP3: nsamp
became 2^64-1, which also masked the MAX_U_32_NUM sentinel and
disabled the file-size fallback.

Store 0 (unknown) for unusable frame counts, MAX_U_32_NUM (unknown)
for failed length queries so the file-size fallback re-engages, and
keep rejecting sample counts above INT_MAX as before. When
mpg123_getstate() does not return MPG123_OK, use the documented
"unknown" value (-1) instead of reading val uninitialized.

Tested on Apple Silicon (LP64): the piped decode now stores the
sentinel instead of 2^64-1; seekable and LAME-tagged inputs are
unchanged (delay/padding 576/1216 read identically); a forged
oversized-Xing file is still rejected; decoded and encoded outputs
are byte-identical.
---
 frontend/get_audio.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/frontend/get_audio.c b/frontend/get_audio.c
index 2898cb3..f492efe 100644
--- a/frontend/get_audio.c
+++ b/frontend/get_audio.c
@@ -2171,22 +2171,30 @@ int lame123_decode_initfile(FILE *fd,
mp3data_struct *mp3data, int *enc_delay, i
     /* Guessing seems to be OK, so we do not have to insist on knowing
        if libmpg123 got that info from Info tag or not. */
     /* I am paranoid about off_t being larger than long or int. */
+    /* mpg123_framelength()/mpg123_length() return MPG123_ERR (-1) when no
+       length is known; the old guards (len <= (unsigned long)-1 and
+       len <= UINT_MAX/2) let that -1 through into the destination fields,
+       and the first cannot reject anything where unsigned long is 64-bit */
     len = mpg123_framelength(global.hip->mh);
-    if(len <= (unsigned long)-1)
-        mp3data->totalframes = len;
+    if(len >= 0 && len <= INT_MAX)
+        mp3data->totalframes = (int)len;
     else
-        return -1;
+        mp3data->totalframes = 0; /* unknown; the mpglib path uses 0
here too */
     len = mpg123_length(global.hip->mh);
-    if(len <= ((unsigned int)-1)/2)
+    if(len >= 0 && len <= ((unsigned int)-1)/2)
         mp3data->nsamp = len;
+    else if(len < 0)
+        mp3data->nsamp = MAX_U_32_NUM; /* unknown: let the file-size
fallback guess */
     else
-        return -1;
+        return -1;  /* implausibly long: keep rejecting, as before */
     /* Encoder delay and padding are not needed when libmpg123 handles gapless
        decoding itself. So let's see if we get away with that. */
-    mpg123_getstate(global.hip->mh, MPG123_ENC_DELAY, &val, NULL);
-    *enc_delay = val;
-    mpg123_getstate(global.hip->mh, MPG123_ENC_PADDING, &val, NULL);
-    *enc_padding = val;
+    if(MPG123_OK != mpg123_getstate(global.hip->mh, MPG123_ENC_DELAY,
&val, NULL))
+        val = -1;           /* documented "unknown" value */
+    *enc_delay = (int)val;
+    if(MPG123_OK != mpg123_getstate(global.hip->mh,
MPG123_ENC_PADDING, &val, NULL))
+        val = -1;
+    *enc_padding = (int)val;
     if(global.in_id3v2_tag)
         free(global.in_id3v2_tag);
     global.in_id3v2_size = 0;