VLC — Three Bugs in One Audit

1. Context

VLC is the most widely used open-source media player in the world — hundreds of millions of installations, capable of playing virtually every audio and video format, reading from files, network streams, DVDs, and Blu-rays. Most of that breadth comes from its parser ecosystem: dozens of demuxers and access modules, each implementing a different format or protocol, each parsing attacker-controlled byte streams. That makes it a genuinely interesting target for vulnerability research.

I audited VLC 4.0-dev (the active development branch) over a period of a few days, focused on the demux and access modules — specifically the code paths that parse container formats, metadata tags, and network tunnel protocols. The approach was pure static analysis: no fuzzing, no instrumentation, just reading the code and following the data. The goal was to find logical errors in bounds checking and arithmetic — the class of bugs that tends to hide in format-specific parsing code that doesn't get the same scrutiny as more central infrastructure.

Three bugs turned up across three different areas. The first is a logic error in the Smooth Streaming manifest parser — a tautological guard that makes an end-of-iterator check meaningless. The second is an integer overflow in the ID3v2.3 tag parser that causes an infinite loop. The third is an integer underflow in the AMT (Automatic Multicast Tunneling) IPv6 handler that moves a buffer pointer roughly 64 KB before its allocation. None of them require any interaction beyond opening the crafted media source.

2. 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).

3. 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;

4. 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.