Confirmed XFMG\VideoInfo\Preparer::fromStream() infinite-recurses on mdat with size 0, OOM-killing SyncAttachmentMirror

mazdas247

Member
Licensed customer
Affected version
v2.3.12 xfmg 2.3.11
Guys please fix this. It's been around for ages. It drives me bananas. My setup has ephemeral containers and so extracts from installer .zip at container start. I can patch but next launch loses my fix and I run into it again.

Affected version
XenForo Media Gallery (version as installed — check Admin > Add-ons)
XenForo 2.3.x

Summary

XFMG\VideoInfo\Preparer::fromStream() enters unbounded recursion on any MP4/MOV containing a top-level box with a declared size of 0, exhausting the PHP memory limit regardless of how high that limit is set.

A box size of 0 is legal per ISO/IEC 14496-12 and means "this box extends to the end of the file." Encoders emit it when writing a stream whose final length isn't known in advance. ffprobe parses such files without complaint.


The relevant code:

src/addons/XFMG/VideoInfo/Preparer.php


Code:
if ($this->isBoxSkippable($ar['boxType']))
{
    fseek($fp, $ar['totalSize'] - 8, SEEK_CUR);
    return $this->fromStream($fp, $parent);
}


With totalSize == 0, the seek becomes fseek($fp, -8, SEEK_CUR) — backwards to the same offset. The method then re-reads the identical header, sees the same box again, and calls itself. PHP has no tail-call elimination, so each pass adds a stack frame; the process grows until memory_limit is exhausted. The reported allocation size in the fatal is 262144 bytes — PHP's VM stack page size, not a data buffer, which is why file size is irrelevant to the failure.

There is also no guard for totalSize < 8, which produces the same backwards-seek loop on a malformed size.

The VideoInfo\Preparer path is unconditional for media_type == 'video' and contains no shell-out, so enabling FFmpeg does not avoid it.

Configuration
  • Mirror attachments to media category enabled
  • MP4/MOV enabled as accepted video formats
  • memory_limit tested at 512 MB, 1 GB, and unlimited
Observed example

A 19 MB, H.264 720p MP4. Box structure:

Code:
         0  ftyp  size=24
        24  moov  size=22104
     22128  mdat  size=0     <- extends to EOF

ffprobe -show_streams reads it without error. Preparer exhausts 512 MB in under a second.

Steps to reproduce
  1. Obtain or create an MP4 whose mdat box declares size 0 (any stream-written encode; ffmpeg piped to stdout produces one).
  2. Attach it to a thread in a forum with attachment mirroring enabled, or run the XFMG:SyncAttachmentMirror rebuild over existing attachments.
  3. Observe the fatal.

Minimal reproduction outside the forum:

$result = (new \XFMG\VideoInfo\Preparer($path))->getInfo();


Actual result

Code:
PHP Fatal error: Allowed memory size of 536870912 bytes exhausted
(tried to allocate 262144 bytes) in src/addons/XFMG/VideoInfo/Preparer.php on line 173

Reached from MirrorCreator::checkTranscodingRequired() via isMirrorable().


Secondary impact: silently stuck rebuild job

Because this path is also reached from the XFMG:SyncAttachmentMirror rebuild job, a single affected attachment blocks that job permanently. The process is killed before it can write to XenForo's server error log; the job's execute_data checkpoint never advances past the offending ID; and the job retries indefinitely. On my installation this ran undetected for well over a year — the only symptom was a persistent "manual rebuild pending" notice in the admin panel and cron entries that appeared healthy. There is no user-visible error and nothing in the XF error log.

Expected result

fromStream()
should handle totalSize == 0 as "extends to EOF" and treat totalSize < 8 as malformed, rather than seeking backwards. The skippable-box path should iterate rather than recurse, so that no input can exhaust the stack. A file the parser can't handle should cause the video to be skipped or rejected, not kill the request or the rebuild job.


Suggested fix


Code:
if ($this->isBoxSkippable($ar['boxType']))
{
    if ($ar['totalSize'] == 0)
    {
        fseek($fp, 0, SEEK_END);
        return false;
    }
    if ($ar['totalSize'] < 8)
    {
        return false;
    }
    fseek($fp, $ar['totalSize'] - 8, SEEK_CUR);
    return $this->fromStream($fp, $parent);
}


Converting the recursion to a loop would additionally harden against any other input that produces a long skip chain.


Sample file


I can supply a reproducing file privately, or you can generate one with any stream-written encode.
 
Last edited:
Back
Top Bottom