Serial-ATA / Serial-ATA/lofty-rs

A stray APE footer discards an entire MPEG file, in every parsing mode

Open
#685 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Rust
Stars
359
Forks
76
Avg merge
12h 54m
Merged PRs (30d)
11

Description

Reproducer

I tried this code:

//! A minimal reproduction: an MP3 with a valid ID3v2 tag and a stray APE footer.

use lofty::config::{ParseOptions, ParsingMode};
use lofty::file::{AudioFile, TaggedFileExt};
use lofty::prelude::{Accessor, ItemKey, TagExt};
use lofty::probe::Probe;
use lofty::tag::{ItemValue, Tag, TagItem, TagType};
use std::fs;
use std::io::Write;
use std::path::Path;

/// Ten MPEG-1 Layer III frames: FF FB 90 00 is 128 kbit/s, 44.1 kHz, stereo, and a
/// frame is then 417 bytes. Padded with 0xAA rather than zeroes, which matters: see
/// the note by the footer below.
fn write_mp3(path: &Path) {
    let mut frame = vec![0xAAu8; 417];
    frame[..4].copy_from_slice(&[0xFF, 0xFB, 0x90, 0x00]);

    let mut bytes = Vec::new();
    for _ in 0..10 {
        bytes.extend_from_slice(&frame);
    }

    fs::write(path, bytes).unwrap();
}

/// The last 32 bytes of the four real files, byte for byte: an APEv2 footer that
/// claims a 142-byte tag with 3 items, where what precedes it is audio.
///
/// The 0xAA padding is what those items are then read out of. Zero padding reads as
/// an item of no length and is accepted, so a file of silence does not reproduce this.
fn append_orphan_ape_footer(path: &Path) {
    let mut footer = Vec::new();
    footer.extend_from_slice(b"APETAGEX");
    footer.extend_from_slice(&2000u32.to_le_bytes()); // version
    footer.extend_from_slice(&142u32.to_le_bytes()); // size, including this footer
    footer.extend_from_slice(&3u32.to_le_bytes()); // item count
    footer.extend_from_slice(&0xA000_0000u32.to_le_bytes()); // footer, has header
    footer.extend_from_slice(&[0u8; 8]); // reserved
    assert_eq!(footer.len(), 32);

    let mut file = fs::OpenOptions::new().append(true).open(path).unwrap();
    file.write_all(&footer).unwrap();
}

fn read(path: &Path, mode: ParsingMode) -> String {
    match Probe::open(path)
        .unwrap()
        .options(ParseOptions::new().parsing_mode(mode))
        .read()
    {
        Ok(tagged) => {
            let title = tagged
                .primary_tag()
                .or_else(|| tagged.first_tag())
                .and_then(|tag| tag.title().map(|t| t.to_string()))
                .unwrap_or_else(|| "<no title>".into());

            format!(
                "Ok — title {title:?}, {} ms",
                tagged.properties().duration().as_millis()
            )
        }
        Err(e) => format!("Err — {e}"),
    }
}

fn main() {
    let dir = std::env::temp_dir().join("lofty-orphan-ape-footer");
    let _ = fs::remove_dir_all(&dir);
    fs::create_dir_all(&dir).unwrap();
    let path = dir.join("song.mp3");

    write_mp3(&path);

    let mut tag = Tag::new(TagType::Id3v2);
    for (key, value) in [
        (ItemKey::TrackTitle, "Winter"),
        (ItemKey::TrackArtist, "Birdy"),
        (ItemKey::AlbumTitle, "Beautiful Lies"),
    ] {
        tag.insert(TagItem::new(key, ItemValue::Text(value.to_string())));
    }
    tag.save_to_path(&path, Default::default()).unwrap();

    println!("lofty 0.24.0\n");
    println!("without the APE footer:");
    for mode in [ParsingMode::Strict, ParsingMode::BestAttempt, ParsingMode::Relaxed] {
        println!("  {mode:?}: {}", read(&path, mode));
    }

    append_orphan_ape_footer(&path);

    println!("\nwith 32 bytes of APE footer appended:");
    for mode in [ParsingMode::Strict, ParsingMode::BestAttempt, ParsingMode::Relaxed] {
        println!("  {mode:?}: {}", read(&path, mode));
    }
}
Summary

An MP3 whose last 32 bytes are an APEv2 footer with no tag body behind it cannot be read at all: Probe::read returns SizeMismatch ("Encountered an invalid item size, either too big or too small to be valid") and everything else about the file is lost — a perfectly valid ID3v2 tag, the stream properties, the duration.

ParsingMode::Relaxed behaves exactly like Strict here, so there is no way to ask lofty to skip the tag it cannot parse and keep the ones it can.

What happens, and what we expected

Four files in a user's library recorded nothing at all. Each has a complete and valid ID3v2.3 tag — 38 frames, title, artist, album, ISRC, MusicBrainz ids, lyrics, a 128 KiB cover — and each ends in an APEv2 footer declaring a 142-byte tag with 3 items, where what precedes the footer is audio. The footer's tag is simply not there; something wrote the footer and not the body.

lofty reads that footer last, in mpeg/read.rs, after everything it has already read correctly:

match crate::ape::tag::read::read_ape_tag(reader, true, parse_options)? {

The ? there discards the whole TaggedFile.

What we expected, at least in BestAttempt and Relaxed, is what the documentation of those modes describes: the offending item is discarded and the parser moves on. A malformed optional trailing tag invalidating a file whose primary tag is valid is surprising — especially since the same file is missing nothing that lofty needs.

We are not suggesting the file is well-formed. It is not: the APE tag is corrupt. The question is whether one corrupt trailing tag should cost the caller everything else in the file.

Reproduction

See the code above. It writes a minimal MPEG file, tags it with lofty itself — so the ID3v2 tag is beyond
question — reads it in all three parsing modes, then appends the 32-byte footer and reads it again.

Two details of the fixture matter:

  • The audio is padded with 0xAA rather than zeroes. An orphan footer is read as items made out of whatever precedes it, and zero padding reads as an item of no length and is accepted — so a file of silence does not reproduce this. The real files are 0xAA padded, which reads as an item of 2 863 311 530 bytes.
  • The footer is byte-for-byte the one those four files carry: version 2000, size 142, item count 3, flags 0xA0000000.
Output
lofty 0.24.0

without the APE footer:
  Strict: Ok — title "Winter", 261 ms
  BestAttempt: Ok — title "Winter", 261 ms
  Relaxed: Ok — title "Winter", 261 ms

with 32 bytes of APE footer appended:
  Strict: Err — Encountered an invalid item size, either too big or too small to be valid
  BestAttempt: Err — Encountered an invalid item size, either too big or too small to be valid
  Relaxed: Err — Encountered an invalid item size, either too big or too small to be valid
For comparison

Mutagen also refuses the APE tag on these files — APEBadItemError: 'w\x16h8' is not a valid APEv2 key — which we take as independent confirmation that the tag really is corrupt. The difference is that reading the ID3 tags of the same file with Mutagen succeeds: the broken APE tag only fails the caller who asks for the APE tag.

Context

Found while scanning a library of 11 291 files, of which these four were the only ones affected. Removing the 32-byte footer — nothing else — makes all four read completely in unmodified lofty, with identical ID3 frames and identical durations, so the audio and the ID3v2 tag were never in question.

Happy to test a patch against the real files.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in mpeg/read.rs at the read_ape_tag(reader, true, parse_options)? call, then follow the supplied reproducer and the APE reader behavior for the orphan footer. Done means BestAttempt and Relaxed preserve the valid ID3v2 tag and MPEG properties instead of returning SizeMismatch; use the reproducer to verify all parsing modes.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
56/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.