VLC — Three Bugs in One Audit

Three distinct vulnerabilities found during a static audit of VLC 4.0-dev: an iterator OOB dereference in the Smooth Streaming parser, an integer overflow that freezes VLC permanently on a crafted ID3 tag, and a uint16_t underflow in the AMT IPv6 tunnel handler that moves a buffer pointer ~64 KB before its allocation.

1. SmoothParser — tautological guard, OOB iterator dereference

The Smooth Streaming parser in modules/demux/smooth/playlist/SmoothParser.cpp builds a segment timeline from <c> (chunk) elements inside a <StreamIndex>. When a chunk element doesn't have a d (duration) attribute, the parser tries to compute the duration from the difference between the current and next chunk's t (timestamp) attributes. The logic that is supposed to check "is there a next element" is wrong:

for(it = chunks.begin(); it != chunks.end(); ++it)
{
    const Node *chunk = *it;

    if(chunk->hasAttribute("d"))
    {
        cur.duration = Integer<uint64_t>(chunk->getAttributeValue("d"));
    }
    else
    {
        if(it != chunks.end())           // Line 95 — always true inside the loop
        {
            const Node *nextchunk = *(it + 1);   // Line 97 — reads chunks.end()
            cur.duration = Integer<uint64_t>(nextchunk->getAttributeValue("t"))
                         - Integer<uint64_t>(chunk->getAttributeValue("t"));
        }
    }
}

The guard if(it != chunks.end()) is a tautology — the loop body only executes when it != chunks.end(), so the condition is always true inside the loop. The intended check was if(std::next(it) != chunks.end()). When the last <c> element lacks a d attribute, it points to the last valid element, the guard passes, and *(it + 1) dereferences chunks.end() — reading 8 bytes from the memory immediately following the vector's internal pointer array.

The resulting garbage pointer is used as a Node* and getAttributeValue("t") is called on it. adaptive::xml::Node has no virtual methods, so getAttributeValue is a direct call that immediately dereferences this->attributes (offset +24 into the garbage pointer). This reliably causes a SIGSEGV in the vast majority of executions. With a controlled heap layout, the read from chunks.end() could yield an attacker-placed fake Node, enabling heap disclosure.

Triggering it requires a crafted .ism Smooth Streaming manifest with at least two <c> elements where the last one omits the d attribute:

<c n="0" d="20000000" t="0"/>
<c n="1" t="20000000"/>  <!-- no d attribute → triggers OOB -->

The fix is two lines: replace the guard it != chunks.end() with std::next(it) != chunks.end(), and replace the dereference *(it + 1) with *std::next(it).

Opening a crafted manifest from the network — via a URL or an embedded player — is enough to trigger the crash. No interaction beyond opening the file is required.

2. ID3v2.3 — uint32_t overflow → infinite loop

The ID3v2.3 frame parser in modules/meta_engine/ID3Tag.h reads each frame's size field and adds 10 to account for the frame header. The result is stored in a uint32_t:

while( i_ID3size > 10 )
{
    uint32_t i_tagname = VLC_FOURCC( p_frame[0], p_frame[1], p_frame[2], p_frame[3] );
    uint32_t i_framesize = ID3TAG_ReadSize( &p_frame[4], i_ID3major != 3 ) + 10;

    if( i_framesize > i_ID3size )
        return 0;

    if( i_framesize > 10 &&
        pf_callback(...) != VLC_SUCCESS )
        break;

    p_frame   += i_framesize;    // no advance if i_framesize == 0
    i_ID3size -= i_framesize;    // no decrement if i_framesize == 0
}

If the 4-byte size field in the crafted tag is 0xFFFFFFF6, then ID3TAG_ReadSize() returns 0xFFFFFFF6 and adding 10 wraps to zero as a uint32_t. The overflow check if(0 > i_ID3size) passes (false), the callback guard if(0 > 10) also passes (false), and both p_frame += 0 and i_ID3size -= 0 leave the loop state unchanged. The condition i_ID3size > 10 remains true. VLC spins at 100% CPU until killed.

Any file format that embeds ID3 tags — MP3, MP4, and several others — can carry this payload. The fix separates the raw size read from the addition and guards before adding:

uint32_t i_rawsize = ID3TAG_ReadSize( &p_frame[4], i_ID3major != 3 );
if( i_rawsize > i_ID3size - 10 )   /* also guards against overflow */
    return 0;
uint32_t i_framesize = i_rawsize + 10;

3. AMT IPv6 — uint16_t underflow → heap OOB

The AMT (Automatic Multicast Tunneling) access module in modules/access/amt.c receives packets from an AMT relay over UDP. For fragmented IPv6 packets, it subtracts 8 from payload_len to account for the IPv6 Fragment Extension Header. Both payload_len and the subtraction result are uint16_t. If payload_len is less than 8, the subtraction underflows:

/* payload_len read directly from attacker-controlled IPv6 header */
payload_len = pkt->p_buffer[AMT_HDR_LEN + 4] << 8;
payload_len += pkt->p_buffer[AMT_HDR_LEN + 5];

...

else if ( b_is_fragmented )
{
    payload_len -= 8;   /* uint16_t: 4 - 8 = 65532 */
}

The guard that should catch this is:

if ( !payload_len || (shift < tunnel && !b_is_fragmented) )
    goto error;

The !payload_len check is false (65532 is nonzero). The shift < tunnel sub-check is explicitly skipped for fragmented packets due to && !b_is_fragmented. Both arms fail, the guard never fires, and execution continues.

Further down, shift = len - payload_len computes a very negative value (60 - 65532 = -65472 as a signed difference), and then:

pkt->p_buffer += shift;        // p_buffer moves ~64 KB before the allocation
pkt->i_buffer = payload_len;   // claims 65532 bytes available

The block is returned to the demuxer with a pointer ~64 KB before its actual allocation and a length of 65532. Any demuxer read or write into pkt->p_buffer for pkt->i_buffer bytes accesses unallocated heap memory.

Triggering this requires an attacker-controlled AMT relay — the victim opens a multicast stream URL that routes through the relay. The relay performs the normal AMT handshake, then sends a crafted subsequent IPv6 fragment with Payload Length = 4 in the IPv6 header. AMT support is only present in VLC 4.0-dev; the 3.0.x release series is not affected.

Triggering this requires an attacker-controlled AMT relay. There are two clean fixes: either add a payload_len < 8 check at each subtraction site, or remove the && !b_is_fragmented exception from the guard so it catches absurd payload lengths for fragmented packets too.