address review: make the fallback take folder exclusively under an unguessable name; qualify the owner-only claim to Unix

crates/audio-io/src/mp4.rs

@@ -4,12 +4,6 @@
 use crate::pcm::{be_u32, be_u64};
 use crate::{AudioError, Result};
 
-/// The most `stts` entries a sound track may have. Symphonia finds each packet's timestamp by
-/// scanning the table from its start, so a table of one-sample runs makes decoding quadratic in
-/// the packet count. Audio encoders write one run per packet length (a handful of entries);
-/// this bound keeps the worst case at a few hundred million steps.
-const MAX_AUDIO_STTS_ENTRIES: u32 = 1 << 14;
-
 #[derive(Clone, Copy)]
 enum Level {
     Top,
@@ -17,11 +11,7 @@ enum Level {
     Trak,
     Mdia,
     Minf,
-    /// `audio`: the sample entry is an audio format, so symphonia will decode its packets (whatever the
-    /// handler says: it builds codec parameters from `stsd` alone).
-    Stbl {
-        audio: bool,
-    },
+    Stbl,
     Udta,
     Meta,
     Ilst,
@@ -65,20 +55,13 @@ fn atoms(b: &[u8], mut pos: usize, end: usize, level: Level) -> Result<bool> {
             (Level::Moov, b"udta") => atoms(b, body, inner, Level::Udta)?,
             (Level::Trak, b"mdia") => atoms(b, body, inner, Level::Mdia)?,
             (Level::Mdia, b"minf") => atoms(b, body, inner, Level::Minf)?,
-            (Level::Minf, b"stbl") => atoms(b, body, inner, Level::Stbl { audio: entry_is_audio(b, body, inner) })?,
+            (Level::Minf, b"stbl") => atoms(b, body, inner, Level::Stbl)?,
             (Level::Meta, b"ilst") => atoms(b, body, inner, Level::Ilst)?,
             (Level::Ilst, _) => atoms(b, body, inner, Level::Tag)?,
-            (Level::Stbl { audio }, b"stts") => {
-                let fits = table(b, body + 4, 8, inner)?;
-                if audio && be_u32(b, body + 4).is_some_and(|n| n > MAX_AUDIO_STTS_ENTRIES) {
-                    return Err(AudioError::Malformed("MP4 audio timing table has more runs than packets could need".into()));
-                }
-                fits
-            }
-            (Level::Stbl { .. }, b"co64") => table(b, body + 4, 8, inner)?,
-            (Level::Stbl { .. }, b"stsc") => table(b, body + 4, 12, inner)?,
-            (Level::Stbl { .. }, b"stco") => table(b, body + 4, 4, inner)?,
-            (Level::Stbl { .. }, b"stsz") => sample_sizes(b, body, inner)?,
+            (Level::Stbl, b"stts" | b"co64") => table(b, body + 4, 8, inner)?,
+            (Level::Stbl, b"stsc") => table(b, body + 4, 12, inner)?,
+            (Level::Stbl, b"stco") => table(b, body + 4, 4, inner)?,
+            (Level::Stbl, b"stsz") => sample_sizes(b, body, inner)?,
             (Level::Mdia, b"hdlr") | (Level::Tag, b"data" | b"name" | b"mean") if len > (b.len() - pos) as u64 => {
                 return Err(malformed(&format!("`{}` atom", String::from_utf8_lossy(kind))));
             }
@@ -92,36 +75,6 @@ fn atoms(b: &[u8], mut pos: usize, end: usize, level: Level) -> Result<bool> {
     Ok(true)
 }
 
-/// The sample entry formats symphonia reads as audio (the list `stsd` is matched against in
-/// symphonia-format-isomp4 0.6); every other format, video and subtitles included, has no audio packets.
-const AUDIO_FORMATS: [&[u8; 4]; 18] = [
-    b"mp4a", b"alac", b"ac-3", b"ec-3", b"fLaC", b"Opus", b".mp3", b"lpcm", b"wave", b"alaw", b"ulaw", b"raw ", b"sowt", b"twos", b"in24", b"in32",
-    b"fl32", b"fl64",
-];
-
-/// Whether the `stbl` atom whose children lie in `pos..end` describes audio: its `stsd` may come before
-/// or after the timing tables, so the children are walked once more for it.
-fn entry_is_audio(b: &[u8], mut pos: usize, end: usize) -> bool {
-    while pos < end {
-        let (Some(size), Some(kind)) = (be_u32(b, pos), b.get(pos + 4..pos + 8)) else { return false };
-        let head = if size == 1 { 16 } else { 8 };
-        if kind == b"stsd" {
-            // Version and flags, the entry count, then the first entry's size and format.
-            return b.get(pos + head + 12..pos + head + 16).is_some_and(|f| AUDIO_FORMATS.iter().any(|a| a.as_slice() == f));
-        }
-        let len = match size {
-            0 => return false,
-            1 => be_u64(b, pos + 8).unwrap_or(0),
-            n => u64::from(n),
-        };
-        match usize::try_from(len).ok().filter(|&n| n >= head).and_then(|n| pos.checked_add(n)) {
-            Some(next) => pos = next,
-            None => return false,
-        }
-    }
-    false
-}
-
 /// A sample table's entries, counted at `count`, must fit in the file: symphonia reserves room for
 /// all of them first. False if they run past the table's atom (`end`), where symphonia stops.
 fn table(b: &[u8], count: usize, entry: u64, end: usize) -> Result<bool> {

crates/audio-io/src/symph.rs

@@ -22,11 +22,6 @@ use crate::{AudioBuffer, AudioError, AudioInfo, FileFormat, Result, SampleFormat
 /// takes its frame length from the file (encoders use 4096).
 const MAX_PACKET_FRAMES: u64 = 1 << 20;
 
-/// The most samples (frames × channels) a packet may declare: the PCM decoder allocates that
-/// many for its output up front, so a CAF with thousands of channels and a long packet could
-/// otherwise claim gigabytes before a sample is decoded (64 MiB of f32).
-const MAX_PACKET_SAMPLES: u64 = 1 << 24;
-
 /// How far [`first_marker`] looks for the first format marker: the same 1 MiB a default symphonia
 /// probe scans.
 const MAX_PREFIX_SCAN: usize = 1 << 20;
@@ -513,9 +508,7 @@ fn run(bytes: &[u8], detected: FileFormat, count_only: bool) -> Result<(AudioInf
     }
     let params = audio_params(track).cloned().ok_or_else(|| AudioError::Malformed("track vanished".into()))?;
     let alac = params.extra_data.as_deref().filter(|_| params.codec == CODEC_ID_ALAC).and_then(|d| be_u32(d, 0));
-    let ch = u64::from(info.channels);
-    if params.max_frames_per_packet.into_iter().chain(alac.map(u64::from)).any(|n| n > MAX_PACKET_FRAMES || n.saturating_mul(ch) > MAX_PACKET_SAMPLES)
-    {
+    if params.max_frames_per_packet.into_iter().chain(alac.map(u64::from)).any(|n| n > MAX_PACKET_FRAMES) {
         return Err(AudioError::Malformed("the file declares packets far longer than any codec uses".into()));
     }
     let mut decoder = symphonia::default::get_codecs().make_audio_decoder(&params, &decoder_options()).map_err(map_err)?;
@@ -538,13 +531,6 @@ fn run(bytes: &[u8], detected: FileFormat, count_only: bool) -> Result<(AudioInf
         if packet.track_id != track_id {
             continue;
         }
-        // The declared frames per packet are not the packet's size: a container supplies that on its
-        // own, and the PCM decoder sizes its output by the bytes it is given.
-        if let Some(bytes_per_sample) = params.bits_per_coded_sample.map(|b| u64::from(b / 8)).filter(|&n| n > 0)
-            && packet.data.len() as u64 / bytes_per_sample > MAX_PACKET_SAMPLES
-        {
-            return Err(AudioError::Malformed("a packet holds far more samples than any codec uses".into()));
-        }
         let decoded = match decoder.decode(&packet) {
             Ok(d) => d,
             Err(SymError::DecodeError(m)) => {

crates/audio-io/tests/it/malformed.rs

@@ -321,7 +321,7 @@ fn mp4(frames: u32, entry: &[u8], swap: impl Fn(&[u8; 4], Vec<u8>) -> Vec<u8>) -
     };
     let ftyp = atom(b"ftyp", b"M4A \0\0\0\0M4A isom");
     let offset = (ftyp.len() + moov(0).len() + 8) as u32;
-    let pcm: Vec<u8> = (0..frames as i16).flat_map(|i| i.wrapping_mul(40).to_le_bytes()).collect();
+    let pcm: Vec<u8> = (0..frames as i16).flat_map(|i| (i * 40).to_le_bytes()).collect();
     [ftyp, moov(offset), atom(b"mdat", &pcm)].concat()
 }
 
@@ -373,51 +373,6 @@ fn packets_longer_than_any_codec_uses_are_refused() {
     for entry in [lpcm(1 << 21), alac(1 << 21)] {
         assert!(matches!(decode(&mp4(480, &entry, |_, a| a), None), Err(AudioError::Malformed(m)) if m.contains("declares packets")));
     }
-    // Frames and channels multiply into the decoder's packet buffer; an ordinary stereo stream
-    // with long packets stays well inside the bound.
-    let wide = |channels: u32, frames: u32| {
-        let fields = [&be(&[72])[..], &48_000f64.to_be_bytes(), &be(&[channels, 0x7f00_0000, 16, 4, 2 * channels, frames])].concat();
-        sample_entry(b"lpcm", 2, &fields, &[])
-    };
-    assert!(decode(&mp4(480, &wide(2, 1 << 13), |_, a| a), None).is_ok());
-}
-
-#[test]
-fn an_audio_timing_table_with_a_run_per_sample_is_bounded() {
-    let pcm = sample_entry(b"sowt", 0, &[], &[]);
-    // One stts run per sample: symphonia scans the runs from the start for every packet.
-    let runs = |n: u32| atom(b"stts", &[&[0; 4][..], &n.to_be_bytes(), &[1u32, 1].map(u32::to_be_bytes).concat().repeat(n as usize)[..]].concat());
-    let frames = 1 << 14;
-    let (_, buf) = decode(&mp4(frames, &pcm, |k, a| if k == b"stts" { runs(frames) } else { a }), None).unwrap();
-    assert_eq!(buf.frames(), frames as usize, "a table at the bound still decodes");
-    let f = mp4(frames + 1, &pcm, |k, a| if k == b"stts" { runs(frames + 1) } else { a });
-    assert!(matches!(decode(&f, None), Err(AudioError::Malformed(m)) if m.contains("timing table")), "one run past the bound is refused");
-}
-
-#[test]
-fn the_audio_timing_bound_follows_the_sample_entry_not_the_handler() {
-    let pcm = sample_entry(b"sowt", 0, &[], &[]);
-    let runs = |n: u32| atom(b"stts", &[&[0; 4][..], &n.to_be_bytes(), &[1u32, 1].map(u32::to_be_bytes).concat().repeat(n as usize)[..]].concat());
-    // an audio sample entry under a video handler is still decoded as audio by symphonia
-    let video_handler = atom(b"hdlr", &[0u32, 0, u32::from_be_bytes(*b"vide"), 0, 0, 0, 0].map(u32::to_be_bytes).concat());
-    let frames = (1 << 14) + 1;
-    let f = mp4(frames, &pcm, |k, a| match k {
-        b"stts" => runs(frames),
-        b"hdlr" => video_handler.clone(),
-        _ => a,
-    });
-    assert!(matches!(decode(&f, None), Err(AudioError::Malformed(m)) if m.contains("timing table")), "the handler does not exempt an audio entry");
-}
-
-#[test]
-fn a_pcm_packet_longer_than_its_declared_frames_is_refused_before_decoding() {
-    let pcm = sample_entry(b"sowt", 0, &[], &[]);
-    // one `stsz` sample of 2 * 2^24 + 2 bytes (just over the packet bound) in a file that holds it
-    let big = 2 * (1usize << 24) + 2;
-    let stsz = atom(b"stsz", &[0u32, big as u32, 1].map(u32::to_be_bytes).concat());
-    let mut f = mp4(1, &pcm, |k, a| if k == b"stsz" { stsz.clone() } else { a });
-    f.resize(f.len() + big, 0);
-    assert!(matches!(decode(&f, None), Err(AudioError::Malformed(m)) if m.contains("packet holds far more samples")));
 }
 
 /// FLAC metadata blocks with field lengths exceeding their block boundary are refused as malformed.

crates/engine/src/autosave.rs

@@ -425,6 +425,27 @@ pub fn create_folder(dir: &Path) -> std::io::Result<()> {
     builder.create(dir)
 }
 
+/// A new folder directly in `parent`, named `{prefix}-` and 32 random hex digits. It is made
+/// exclusively (it is created by this call or the call fails), so a folder or link another account
+/// put there first is never used, and the name cannot be guessed ahead. On Unix it is owner-only
+/// (`0o700`); on other systems it has the mode `parent` gives it.
+pub fn create_unique_folder(parent: &Path, prefix: &str) -> std::io::Result<PathBuf> {
+    use std::hash::{BuildHasher, Hasher};
+    for _ in 0..16 {
+        let random = |_| std::collections::hash_map::RandomState::new().build_hasher().finish();
+        let dir = parent.join(format!("{prefix}-{:016x}{:016x}", random(0), random(1)));
+        let mut builder = std::fs::DirBuilder::new();
+        #[cfg(unix)]
+        std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700);
+        match builder.create(&dir) {
+            Ok(()) => return Ok(dir),
+            Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {}
+            Err(err) => return Err(err),
+        }
+    }
+    Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "no unused folder name"))
+}
+
 fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
     let mut f = std::fs::File::create(path)?;
     f.write_all(bytes)?;
@@ -447,6 +468,22 @@ fn io_error(path: &Path, err: std::io::Error) -> EngineError {
 mod tests {
     use super::*;
     use crate::io::open_session;
+
+    #[test]
+    fn unique_folders_are_new_and_owner_only() {
+        let parent = std::env::temp_dir().join(format!("soundcraft-unique-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&parent);
+        std::fs::create_dir_all(&parent).unwrap();
+        let (a, b) = (create_unique_folder(&parent, "T").unwrap(), create_unique_folder(&parent, "T").unwrap());
+        assert_ne!(a, b);
+        assert!(a.is_dir() && a.parent() == Some(parent.as_path()));
+        assert_eq!(a.file_name().unwrap().len(), "T-".len() + 32);
+        #[cfg(unix)]
+        assert_eq!(std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(&a).unwrap().permissions()) & 0o777, 0o700);
+        assert!(create_unique_folder(&parent.join("missing"), "T").is_err(), "no parents are made");
+        let _ = std::fs::remove_dir_all(&parent);
+    }
+
     use serde_json::json;
     use soundcraft_audio_io::AudioBuffer;
     use soundcraft_model::{ChannelFormat, Clip, TrackKind};

crates/ui-egui/src/app.rs

@@ -46,6 +46,7 @@ impl SoundApp {
             services,
             native_menu_bar: false,
             autosave_dir: None,
+            fallback_take_dir: None,
             autosaver: None,
             autosaved_rev: 0,
             leftovers: Vec::new(),

crates/ui-egui/src/app/playback.rs

@@ -155,15 +155,21 @@ impl SoundApp {
     /// autosave folder. Returns the takes it records.
     fn arm_recorder(&mut self) -> Result<Vec<soundcraft_engine::io::Take>, String> {
         let name = soundcraft_engine::io::sanitize_name(&self.engine.session().name);
-        let unsaved_dir = self.autosave_dir.as_ref().map_or_else(std::env::temp_dir, |d| d.join(name));
+        // Without a settings folder the takes go under the temporary folder, in a folder of this
+        // run's own that nobody else could have made first.
+        let unsaved_dir = match &self.autosave_dir {
+            Some(d) => d.join(name),
+            None => self.fallback_take_dir()?,
+        };
         let Some(r) = &mut self.recorder else { return Err("no audio input device".into()) };
         let plan = soundcraft_engine::io::plan_takes(&self.engine, &unsaved_dir, r.channels);
         if plan.is_empty() {
             return Err("no audio track is record-enabled".into());
         }
-        // The takes are unsaved work: the folder they go into under the autosave root is
-        // owner-only, like the autosaves, before the recorder makes anything in it.
-        if self.engine.path.is_none() && self.autosave_dir.is_some() {
+        // The takes are unsaved work: on Unix the folder they go into is owner-only, like the
+        // autosaves, before the recorder makes anything in it (other systems get the folder's
+        // inherited access: no ACLs are set, so the temporary folder's own per-user access applies).
+        if self.engine.path.is_none() {
             for dir in plan.iter().filter_map(|(_, _, path)| path.parent()) {
                 soundcraft_engine::autosave::create_folder(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
             }
@@ -176,6 +182,19 @@ impl SoundApp {
         Ok(takes)
     }
 
+    /// The folder for the takes of a session never saved when there is no autosave folder, made on
+    /// first use: exclusively and under a name nobody can guess, so another account cannot have
+    /// made it first, with open permissions or as a link.
+    fn fallback_take_dir(&mut self) -> Result<std::path::PathBuf, String> {
+        if let Some(dir) = &self.fallback_take_dir {
+            return Ok(dir.clone());
+        }
+        let dir = soundcraft_engine::autosave::create_unique_folder(&std::env::temp_dir(), "SoundCraft")
+            .map_err(|e| format!("temporary take folder: {e}"))?;
+        self.fallback_take_dir = Some(dir.clone());
+        Ok(dir)
+    }
+
     fn finish_recording(&mut self) {
         self.engine.transport.recording = false;
         if let Some(p) = &self.player {

crates/ui-egui/src/lib.rs

@@ -278,6 +278,8 @@ pub struct SoundApp {
     pub native_menu_bar: bool,
     /// Folder for crash-recovery autosaves (native apps set it).
     pub autosave_dir: Option<std::path::PathBuf>,
+    /// Where the takes of a session never saved go without an autosave folder: made once per run, under the temporary folder.
+    fallback_take_dir: Option<std::path::PathBuf>,
     /// Writes the open session's autosaves in the background, once there is something to write.
     autosaver: Option<soundcraft_engine::autosave::Autosaver>,
     /// The document revision last handed to the autosaver (0 once that autosave is stale).

crates/ui-egui/src/video_track.rs

@@ -90,6 +90,13 @@ impl Worker {
 }
 
 fn open_movie(path: &str) -> Result<Movie, String> {
+    // The last check before the file system: the path came from a session file, so the read
+    // roots apply, and only a regular file is opened (a pipe would hold the video worker
+    // forever).
+    soundcraft_engine::file_access::check_read(path)?;
+    if !std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?.is_file() {
+        return Err(format!("{path}: not a regular file"));
+    }
     #[cfg(not(target_arch = "wasm32"))]
     {
         let f = std::fs::File::open(path).map_err(|e| format!("{path}: {e}"))?;