From 69c032f920d518e481da98cb5a495460e753cdf4 Mon Sep 17 00:00:00 2001 From: salmonik Date: Wed, 4 Nov 2026 01:07:53 +0100 Subject: utilities: sanitize filenames and isolate temp operations --- src/commands/utilities.rs | 153 +++++++++++++++++++++++++++++++--------------- 1 file changed, 105 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/commands/utilities.rs b/src/commands/utilities.rs index 79f4508..2179ce9 100644 --- a/src/commands/utilities.rs +++ b/src/commands/utilities.rs @@ -66,15 +66,47 @@ pub(crate) fn stem_of(filename: &str) -> &str { } } -fn temp_path_for(name: &str) -> String { - format!("{}/{}", crate::config::TEMP_PATH, name) +/// A safe basename: no directory components, no characters Discord/Copyparty +/// filenames commonly abuse, and never `.`/`..`. +fn safe_basename(name: &str) -> String { + // Treat both separators as directory boundaries: Discord filenames may + // come from Windows clients even though the bot runs on Linux. + let base = name.rsplit(['/', '\\']).next().unwrap_or(name); + let cleaned = sanitize_filename(base) + .trim_matches(|c| c == '.' || c == ' ') + .to_string(); + if cleaned.is_empty() { + "file".to_string() + } else { + cleaned + } } -/// Best-effort removal of temp files; ignores missing files. -async fn cleanup(paths: &[&str]) { - for p in paths { - let _ = tokio::fs::remove_file(p).await; - } +/// Create a unique per-operation directory and return +/// `(directory, input_path, output_path)`. Concurrent commands can never +/// overwrite or delete each other's files. +async fn temp_op( + original: &str, + output_name: &str, +) -> std::io::Result<(String, String, String)> { + let safe = safe_basename(original); + let ext = ext_of(&safe) + .map(|ext| format!(".{ext}")) + .unwrap_or_default(); + let dir = format!( + "{}/op_{}", + crate::config::TEMP_PATH, + uuid::Uuid::new_v4() + ); + tokio::fs::create_dir_all(&dir).await?; + let input_path = format!("{dir}/input{ext}"); + let output_path = format!("{dir}/{}", safe_basename(output_name)); + Ok((dir, input_path, output_path)) +} + +/// Best-effort removal of a per-operation temp directory. +async fn cleanup_dir(dir: &str) { + let _ = tokio::fs::remove_dir_all(dir).await; } /// Resolve the input source — either an autocomplete-picked copyparty file or @@ -234,35 +266,32 @@ enum DeliverKind { /// file, returning the encoded bytes and the output filename. /// /// Mirrors the per-attachment path of Python's `Compression` message command, -/// including sequential `TEMP_PATH` reuse so identical attachment stems don't -/// collide (each iteration writes, encodes, then `cleanup`s before the next). +/// but each invocation gets its own temp directory, so concurrent calls cannot +/// overwrite or delete each other's files. async fn compress_attachment_bytes( ft: FileType, original: &str, input_bytes: &[u8], ) -> Option<(Vec, String)> { - let stem = stem_of(original).to_string(); - let out_ext = compress_output_ext(ft, original); + let safe_original = safe_basename(original); + let stem = stem_of(&safe_original).to_string(); + let out_ext = compress_output_ext(ft, &safe_original); let out_filename = format!("compressed_{stem}.{out_ext}"); - let input_path = temp_path_for(original); - let output_path = temp_path_for(&out_filename); - let cleanup_paths = [input_path.as_str(), output_path.as_str()]; + let (dir, input_path, output_path) = temp_op(&safe_original, &out_filename).await.ok()?; if tokio::fs::write(&input_path, input_bytes).await.is_err() { + cleanup_dir(&dir).await; return None; } - let mut cmd = match build_compress_command(ft, &input_path, &output_path) { - Some(c) => c, - None => { - cleanup(&cleanup_paths).await; - return None; - } + let Some(mut cmd) = build_compress_command(ft, &input_path, &output_path) else { + cleanup_dir(&dir).await; + return None; }; let res = match cmd.output().await { Ok(out) if out.status.success() => tokio::fs::read(&output_path).await.ok(), _ => None, }; - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; res.map(|b| (b, out_filename)) } @@ -270,17 +299,14 @@ async fn compress_attachment_bytes( /// Uses Vorbis instead of Opus because symphonia 0.6 lacks an Opus decoder. /// Returns the compressed .ogg bytes on success. pub(crate) async fn compress_audio_bytes(input_bytes: &[u8], original_name: &str) -> Option> { - let safe_name = sanitize_filename(original_name); - let safe_name = if safe_name.is_empty() { "audio".to_string() } else { safe_name }; + let safe_name = safe_basename(original_name); let stem = stem_of(&safe_name).to_string(); let out_filename = format!("compressed_{stem}.ogg"); - let input_path = temp_path_for(&safe_name); - let output_path = temp_path_for(&out_filename); - let cleanup_paths = [input_path.as_str(), output_path.as_str()]; + let (dir, input_path, output_path) = temp_op(&safe_name, &out_filename).await.ok()?; - let _ = tokio::fs::create_dir_all(crate::config::TEMP_PATH).await; if let Err(e) = tokio::fs::write(&input_path, input_bytes).await { tracing::warn!("compress_audio_bytes: failed to write input: {e}"); + cleanup_dir(&dir).await; return None; } tracing::debug!( @@ -324,7 +350,7 @@ pub(crate) async fn compress_audio_bytes(input_bytes: &[u8], original_name: &str None } }; - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; res } @@ -608,16 +634,20 @@ pub async fn compress( None => return Ok(()), }; - let ft = file_type(&original_filename); - let stem = stem_of(&original_filename).to_string(); - let out_ext = compress_output_ext(ft, &original_filename); + let safe_original = safe_basename(&original_filename); + let ft = file_type(&safe_original); + let stem = stem_of(&safe_original).to_string(); + let out_ext = compress_output_ext(ft, &safe_original); let out_filename = format!("compressed_{stem}.{out_ext}"); - let input_path = temp_path_for(&original_filename); - let output_path = temp_path_for(&out_filename); - let cleanup_paths = [input_path.as_str(), output_path.as_str()]; + let Some((dir, input_path, output_path)) = temp_op(&safe_original, &out_filename).await.ok() + else { + ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?; + return Ok(()); + }; if let Err(e) = tokio::fs::write(&input_path, &input_bytes).await { tracing::warn!("compress: failed to write input: {e}"); + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?; return Ok(()); } @@ -625,7 +655,7 @@ pub async fn compress( let result = match build_compress_command(ft, &input_path, &output_path) { Some(mut cmd) => cmd.output().await, None => { - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_UNSUPPORTED_TYPE).await?; return Ok(()); } @@ -634,7 +664,7 @@ pub async fn compress( match result { Ok(out) if out.status.success() => match tokio::fs::read(&output_path).await { Ok(out_bytes) => { - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; deliver( &ctx, out_bytes, @@ -646,12 +676,12 @@ pub async fn compress( } Err(e) => { tracing::warn!("compress: output missing after encode: {e}"); - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?; } }, _ => { - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?; } } @@ -689,15 +719,19 @@ pub async fn convert( return Ok(()); } - let ft = file_type(&original_filename); - let stem = stem_of(&original_filename).to_string(); + let safe_original = safe_basename(&original_filename); + let ft = file_type(&safe_original); + let stem = stem_of(&safe_original).to_string(); let out_filename = format!("{stem}.{fmt}"); - let input_path = temp_path_for(&original_filename); - let output_path = temp_path_for(&out_filename); - let cleanup_paths = [input_path.as_str(), output_path.as_str()]; + let Some((dir, input_path, output_path)) = temp_op(&safe_original, &out_filename).await.ok() + else { + ctx.say(lang::L_UTIL_CONVERT_FAILED).await?; + return Ok(()); + }; if let Err(e) = tokio::fs::write(&input_path, &input_bytes).await { tracing::warn!("convert: failed to write input: {e}"); + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_CONVERT_FAILED).await?; return Ok(()); } @@ -714,7 +748,7 @@ pub async fn convert( c } FileType::Unknown => { - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_UNSUPPORTED_TYPE).await?; return Ok(()); } @@ -723,7 +757,7 @@ pub async fn convert( match cmd.output().await { Ok(out) if out.status.success() => match tokio::fs::read(&output_path).await { Ok(out_bytes) => { - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; deliver( &ctx, out_bytes, @@ -735,17 +769,17 @@ pub async fn convert( } Err(e) => { tracing::warn!("convert: output missing after encode: {e}"); - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::L_UTIL_CONVERT_FAILED).await?; } }, Ok(out) => { let stderr = String::from_utf8_lossy(&out.stderr); - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::fmt_util_convert_err(&stderr)).await?; } Err(e) => { - cleanup(&cleanup_paths).await; + cleanup_dir(&dir).await; ctx.say(lang::fmt_util_convert_err(&e.to_string())).await?; } } @@ -1017,3 +1051,26 @@ pub fn commands() -> Vec> { message_download(), ] } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn safe_basename_strips_directories() { + assert_eq!(safe_basename("/music/foo.mp3"), "foo.mp3"); + assert_eq!(safe_basename("../../etc/passwd"), "passwd"); + assert_eq!( + safe_basename(r"..\..\windows\system32\evil.exe"), + "evil.exe" + ); + assert_eq!(safe_basename(r"C:\Users\x\song.mp3"), "song.mp3"); + } + + #[test] + fn safe_basename_never_returns_dot_entries() { + assert_eq!(safe_basename(".."), "file"); + assert_eq!(safe_basename("."), "file"); + assert_eq!(safe_basename(""), "file"); + } +} -- cgit v1.2.3