lashman/pixstrip
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix 40+ bugs from audit passes 9-12

Browse Source 
- PNG chunk parsing overflow protection with checked arithmetic
- Font directory traversal bounded with global result limit
- find_unique_path TOCTOU race fixed with create_new + marker byte
- Watch mode "processed" dir exclusion narrowed to prevent false skips
- Metadata copy now checks format support before little_exif calls
- Clipboard temp files cleaned up on app exit
- Atomic writes for file manager integration scripts
- BMP format support added to encoder and convert step
- Regex DoS protection with DFA size limit
- Watermark NaN/negative scale guard
- Selective EXIF stripping for privacy/custom metadata modes
- CLI watch mode: file stability checks, per-file history saves
- High contrast toggle preserves and restores original theme
- Image list deduplication uses O(1) HashSet lookups
- Saturation/trim/padding overflow guards in adjustments
This commit is contained in:
lashman
2026-03-07 22:14:48 +02:00
parent adef810691
commit d1cab8a691
18 changed files with 600 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
pixstrip-core/src/encoder.rs
View File

@@ -44,6 +44,7 @@ impl OutputEncoder {
ImageFormat::Avif => self.encode_avif(img, quality.unwrap_or(80)),
ImageFormat::Gif => self.encode_fallback(img, image::ImageFormat::Gif),
ImageFormat::Tiff => self.encode_fallback(img, image::ImageFormat::Tiff),
ImageFormat::Bmp => self.encode_fallback(img, image::ImageFormat::Bmp),
}
}
@@ -68,7 +69,7 @@ impl OutputEncoder {
ImageFormat::WebP => preset.webp_quality() as u8,
ImageFormat::Avif => preset.avif_quality() as u8,
ImageFormat::Png => preset.png_level(),
_ => preset.jpeg_quality(),
ImageFormat::Gif | ImageFormat::Tiff | ImageFormat::Bmp => preset.jpeg_quality(),
}
}
@@ -202,6 +203,11 @@ impl OutputEncoder {
/// The pHYs chunk must appear before the first IDAT chunk.
/// DPI is converted to pixels per meter (1 inch = 0.0254 meters).
fn insert_png_phys_chunk(png_data: &[u8], dpi: u32) -> Vec<u8> {
// Not a valid PNG (too short for signature) - return as-is
if png_data.len() < 8 {
return png_data.to_vec();
}
// PNG pixels per meter = DPI / 0.0254
let ppm = (dpi as f64 / 0.0254).round() as u32;
@@ -241,9 +247,12 @@ fn insert_png_phys_chunk(png_data: &[u8], dpi: u32) -> Vec<u8> {
png_data[pos], png_data[pos + 1], png_data[pos + 2], png_data[pos + 3],
]) as usize;
let chunk_type = &png_data[pos + 4..pos + 8];
let total_chunk_size = 4 + 4 + chunk_len + 4; // len + type + data + crc
// Use checked arithmetic to prevent overflow on malformed PNGs
let Some(total_chunk_size) = chunk_len.checked_add(12) else {
break; // chunk_len so large it overflows - malformed PNG
};
if pos + total_chunk_size > png_data.len() {
if pos.checked_add(total_chunk_size).map_or(true, |end| end > png_data.len()) {
break;
}
@@ -263,6 +272,11 @@ fn insert_png_phys_chunk(png_data: &[u8], dpi: u32) -> Vec<u8> {
pos += total_chunk_size;
}
// Copy any remaining bytes (e.g. trailing data after a truncated chunk)
if pos < png_data.len() {
result.extend_from_slice(&png_data[pos..]);
}
result
}

67
pixstrip-core/src/executor.rs
View File

@@ -176,16 +176,6 @@ impl PipelineExecutor {
.unwrap_or("unknown")
.to_string();
let current = completed_ref.fetch_add(1, Ordering::Relaxed) + 1;
let _ = tx_clone.send(ProgressUpdate {
current,
total,
current_file: file_name.clone(),
succeeded_so_far: succeeded_ref.load(Ordering::Relaxed),
failed_so_far: failed_ref.load(Ordering::Relaxed),
});
let loader = ImageLoader::new();
let encoder = OutputEncoder::with_options(EncoderOptions {
progressive_jpeg: job.progressive_jpeg,
@@ -207,13 +197,23 @@ impl PipelineExecutor {
Err(e) => {
failed_ref.fetch_add(1, Ordering::Relaxed);
if let Ok(mut errs) = errors_ref.lock() {
errs.push((file_name, e.to_string()));
errs.push((file_name.clone(), e.to_string()));
}
if pause_on_error {
pause_flag.store(true, Ordering::Relaxed);
}
}
}
// Send progress after processing so counts are consistent
let current = completed_ref.fetch_add(1, Ordering::Relaxed) + 1;
let _ = tx_clone.send(ProgressUpdate {
current,
total,
current_file: file_name,
succeeded_so_far: succeeded_ref.load(Ordering::Relaxed),
failed_so_far: failed_ref.load(Ordering::Relaxed),
});
});
});
// Drop sender so the receiver loop ends
@@ -493,8 +493,8 @@ impl PipelineExecutor {
} => match output_format {
ImageFormat::Jpeg => jpeg_quality.unwrap_or(85),
ImageFormat::Png => png_level.unwrap_or(6),
ImageFormat::WebP => webp_quality.map(|q| q as u8).unwrap_or(80),
ImageFormat::Avif => avif_quality.map(|q| q as u8).unwrap_or(50),
ImageFormat::WebP => webp_quality.map(|q| q.clamp(0.0, 100.0) as u8).unwrap_or(80),
ImageFormat::Avif => avif_quality.map(|q| q.clamp(0.0, 100.0) as u8).unwrap_or(50),
_ => 85,
},
});
@@ -599,22 +599,33 @@ impl PipelineExecutor {
// KeepAll: copy everything back from source.
// Privacy/Custom: copy metadata back, then selectively strip certain tags.
// StripAll: do nothing (already stripped by re-encoding).
// Note: little_exif only supports JPEG and TIFF metadata manipulation.
if let Some(ref meta_config) = job.metadata {
let format_supports_exif = matches!(
output_format,
ImageFormat::Jpeg | ImageFormat::Tiff
);
match meta_config {
crate::operations::MetadataConfig::KeepAll => {
if !copy_metadata_from_source(&source.path, &output_path) {
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
if format_supports_exif {
if !copy_metadata_from_source(&source.path, &output_path) {
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
}
}
// For non-JPEG/TIFF formats, metadata is lost during re-encoding
// and cannot be restored. This is a known limitation.
}
crate::operations::MetadataConfig::StripAll => {
// Already stripped by re-encoding - nothing to do
}
_ => {
// Privacy or Custom: copy all metadata back, then strip unwanted tags
if !copy_metadata_from_source(&source.path, &output_path) {
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
if format_supports_exif {
if !copy_metadata_from_source(&source.path, &output_path) {
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
}
strip_selective_metadata(&output_path, meta_config);
}
strip_selective_metadata(&output_path, meta_config);
}
}
}
@@ -697,8 +708,17 @@ fn paths_are_same(a: &std::path::Path, b: &std::path::Path) -> bool {
match (a.canonicalize(), b.canonicalize()) {
(Ok(ca), Ok(cb)) => ca == cb,
_ => {
// If canonicalize fails (file doesn't exist yet), compare components directly
a.as_os_str() == b.as_os_str()
// If canonicalize fails (output file doesn't exist yet),
// canonicalize parent directories and compare with filename appended
let resolve = |p: &std::path::Path| -> Option<std::path::PathBuf> {
let parent = p.parent()?;
let name = p.file_name()?;
Some(parent.canonicalize().ok()?.join(name))
};
match (resolve(a), resolve(b)) {
(Some(ra), Some(rb)) => ra == rb,
_ => a.as_os_str() == b.as_os_str(),
}
}
}
}
@@ -732,7 +752,12 @@ fn find_unique_path(path: &std::path::Path) -> std::path::PathBuf {
.create_new(true)
.open(&candidate)
{
Ok(_) => return candidate,
Ok(mut f) => {
// Write a marker byte so cleanup_placeholder (which only removes
// 0-byte files) won't delete our reservation before the real write
let _ = std::io::Write::write_all(&mut f, b"~");
return candidate;
}
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(_) => continue,
}

12
pixstrip-core/src/fm_integration.rs
View File

@@ -2,7 +2,7 @@ use std::path::{Path, PathBuf};
use crate::error::Result;
use crate::preset::Preset;
use crate::storage::PresetStore;
use crate::storage::{atomic_write, PresetStore};
/// Supported file managers for right-click integration.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -214,7 +214,7 @@ class PixstripExtension(GObject.GObject, Nautilus.MenuProvider):
bin = escaped_bin,
);
std::fs::write(nautilus_extension_path(), script)?;
atomic_write(&nautilus_extension_path(), &script)?;
Ok(())
}
@@ -259,7 +259,7 @@ fn install_nemo() -> Result<()> {
Mimetypes=image/*;\n",
bin = bin,
);
std::fs::write(nemo_action_path(), open_action)?;
atomic_write(&nemo_action_path(), &open_action)?;
// Per-preset actions
let presets = get_preset_names();
@@ -279,7 +279,7 @@ fn install_nemo() -> Result<()> {
safe_label = shell_safe(name),
bin = bin,
);
std::fs::write(action_path, action)?;
atomic_write(&action_path, &action)?;
}
Ok(())
@@ -361,7 +361,7 @@ fn install_thunar() -> Result<()> {
}
actions.push_str("</actions>\n");
std::fs::write(thunar_action_path(), actions)?;
atomic_write(&thunar_action_path(), &actions)?;
Ok(())
}
@@ -429,7 +429,7 @@ fn install_dolphin() -> Result<()> {
));
}
std::fs::write(dolphin_service_path(), desktop)?;
atomic_write(&dolphin_service_path(), &desktop)?;
Ok(())
}

8
pixstrip-core/src/loader.rs
View File

@@ -34,7 +34,8 @@ impl ImageLoader {
reason: e.to_string(),
})?;
let img = reader.decode().map_err(|e| PixstripError::ImageLoad {
// Read only the image header for dimensions (avoids full decode into RAM)
let (width, height) = reader.into_dimensions().map_err(|e| PixstripError::ImageLoad {
path: path.to_path_buf(),
reason: e.to_string(),
})?;
@@ -45,10 +46,7 @@ impl ImageLoader {
.and_then(ImageFormat::from_extension);
Ok(ImageInfo {
dimensions: Dimensions {
width: img.width(),
height: img.height(),
},
dimensions: Dimensions { width, height },
format,
file_size: metadata.len(),
})

67
pixstrip-core/src/operations/metadata.rs
View File

@@ -16,21 +16,68 @@ pub fn strip_metadata(
MetadataConfig::StripAll => {
strip_all_exif(input, output)?;
}
MetadataConfig::Privacy => {
// Privacy mode strips GPS and camera info but keeps copyright.
// For now, we strip all EXIF as a safe default.
// Selective tag preservation requires full EXIF parsing.
strip_all_exif(input, output)?;
}
MetadataConfig::Custom { .. } => {
// Custom selective stripping - simplified to strip-all for now.
// Full selective stripping requires per-tag EXIF manipulation.
strip_all_exif(input, output)?;
MetadataConfig::Privacy | MetadataConfig::Custom { .. } => {
// Copy file first, then selectively strip using little_exif
std::fs::copy(input, output).map_err(PixstripError::Io)?;
strip_selective_exif(output, config);
}
}
Ok(())
}
fn strip_selective_exif(path: &Path, config: &MetadataConfig) {
use little_exif::exif_tag::ExifTag;
use little_exif::metadata::Metadata;
let Ok(source_meta) = Metadata::new_from_path(path) else {
return;
};
let mut strip_ids: Vec<u16> = Vec::new();
if config.should_strip_gps() {
strip_ids.push(ExifTag::GPSInfo(Vec::new()).as_u16());
}
if config.should_strip_camera() {
strip_ids.push(ExifTag::Make(String::new()).as_u16());
strip_ids.push(ExifTag::Model(String::new()).as_u16());
strip_ids.push(ExifTag::LensModel(String::new()).as_u16());
strip_ids.push(ExifTag::LensMake(String::new()).as_u16());
strip_ids.push(ExifTag::SerialNumber(String::new()).as_u16());
strip_ids.push(ExifTag::LensSerialNumber(String::new()).as_u16());
strip_ids.push(ExifTag::LensInfo(Vec::new()).as_u16());
}
if config.should_strip_software() {
strip_ids.push(ExifTag::Software(String::new()).as_u16());
strip_ids.push(ExifTag::MakerNote(Vec::new()).as_u16());
}
if config.should_strip_timestamps() {
strip_ids.push(ExifTag::ModifyDate(String::new()).as_u16());
strip_ids.push(ExifTag::DateTimeOriginal(String::new()).as_u16());
strip_ids.push(ExifTag::CreateDate(String::new()).as_u16());
strip_ids.push(ExifTag::SubSecTime(String::new()).as_u16());
strip_ids.push(ExifTag::SubSecTimeOriginal(String::new()).as_u16());
strip_ids.push(ExifTag::SubSecTimeDigitized(String::new()).as_u16());
strip_ids.push(ExifTag::OffsetTime(String::new()).as_u16());
strip_ids.push(ExifTag::OffsetTimeOriginal(String::new()).as_u16());
strip_ids.push(ExifTag::OffsetTimeDigitized(String::new()).as_u16());
}
if config.should_strip_copyright() {
strip_ids.push(ExifTag::Copyright(String::new()).as_u16());
strip_ids.push(ExifTag::Artist(String::new()).as_u16());
strip_ids.push(ExifTag::OwnerName(String::new()).as_u16());
}
let mut new_meta = Metadata::new();
for tag in source_meta.data() {
if !strip_ids.contains(&tag.as_u16()) {
new_meta.set_tag(tag.clone());
}
}
let _ = new_meta.write_to_file(path);
}
fn strip_all_exif(input: &Path, output: &Path) -> Result<()> {
let data = std::fs::read(input).map_err(PixstripError::Io)?;
let cleaned = if data.len() >= 8 && &data[..8] == b"\x89PNG\r\n\x1a\n" {

41
pixstrip-core/src/operations/rename.rs
View File

@@ -224,6 +224,18 @@ pub fn apply_case_conversion(name: &str, case_mode: u32) -> String {
}
}
/// Pre-compile a regex for batch use. Returns None (with message) if invalid.
pub fn compile_rename_regex(find: &str) -> Option<regex::Regex> {
if find.is_empty() {
return None;
}
regex::RegexBuilder::new(find)
.size_limit(1 << 16)
.dfa_size_limit(1 << 16)
.build()
.ok()
}
/// Apply regex find-and-replace on a filename
pub fn apply_regex_replace(name: &str, find: &str, replace: &str) -> String {
if find.is_empty() {
@@ -231,6 +243,7 @@ pub fn apply_regex_replace(name: &str, find: &str, replace: &str) -> String {
}
match regex::RegexBuilder::new(find)
.size_limit(1 << 16)
.dfa_size_limit(1 << 16)
.build()
{
Ok(re) => re.replace_all(name, replace).into_owned(),
@@ -238,9 +251,17 @@ pub fn apply_regex_replace(name: &str, find: &str, replace: &str) -> String {
}
}
/// Apply a pre-compiled regex find-and-replace on a filename
pub fn apply_regex_replace_compiled(name: &str, re: &regex::Regex, replace: &str) -> String {
re.replace_all(name, replace).into_owned()
}
pub fn resolve_collision(path: &Path) -> PathBuf {
if !path.exists() {
return path.to_path_buf();
// Use create_new (O_CREAT|O_EXCL) for atomic reservation, preventing TOCTOU races
match std::fs::OpenOptions::new().write(true).create_new(true).open(path) {
Ok(_) => return path.to_path_buf(),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
Err(_) => return path.to_path_buf(), // other errors (e.g. permission) - let caller handle
}
let parent = path.parent().unwrap_or(Path::new("."));
@@ -259,15 +280,21 @@ pub fn resolve_collision(path: &Path) -> PathBuf {
} else {
parent.join(format!("{}_{}.{}", stem, i, ext))
};
if !candidate.exists() {
return candidate;
match std::fs::OpenOptions::new().write(true).create_new(true).open(&candidate) {
Ok(_) => return candidate,
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(_) => continue,
}
}
// Fallback - should never happen with 1000 attempts
// Fallback with timestamp for uniqueness
let ts = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_millis())
.unwrap_or(0);
if ext.is_empty() {
parent.join(format!("{}_overflow", stem))
parent.join(format!("{}_{}", stem, ts))
} else {
parent.join(format!("{}_overflow.{}", stem, ext))
parent.join(format!("{}_{}.{}", stem, ts, ext))
}
}

35
pixstrip-core/src/operations/watermark.rs
View File

@@ -94,7 +94,7 @@ fn find_system_font(family: Option<&str>) -> Result<Vec<u8>> {
.to_lowercase();
if file_name.contains(&name_lower)
&& (file_name.ends_with(".ttf") || file_name.ends_with(".otf"))
&& (file_name.contains("regular") || !file_name.contains("bold") && !file_name.contains("italic"))
&& (file_name.contains("regular") || (!file_name.contains("bold") && !file_name.contains("italic")))
{
if let Ok(data) = std::fs::read(&path) {
return Ok(data);
@@ -136,24 +136,27 @@ fn walkdir(dir: &std::path::Path) -> std::io::Result<Vec<std::path::PathBuf>> {
fn walkdir_depth(dir: &std::path::Path, max_depth: u32) -> std::io::Result<Vec<std::path::PathBuf>> {
const MAX_RESULTS: usize = 10_000;
let mut results = Vec::new();
if max_depth == 0 || !dir.is_dir() {
return Ok(results);
walkdir_depth_inner(dir, max_depth, &mut results, MAX_RESULTS);
Ok(results)
}
fn walkdir_depth_inner(dir: &std::path::Path, max_depth: u32, results: &mut Vec<std::path::PathBuf>, max: usize) {
if max_depth == 0 || !dir.is_dir() || results.len() >= max {
return;
}
for entry in std::fs::read_dir(dir)? {
if results.len() >= MAX_RESULTS {
let Ok(entries) = std::fs::read_dir(dir) else { return };
for entry in entries {
if results.len() >= max {
break;
}
let entry = entry?;
let Ok(entry) = entry else { continue };
let path = entry.path();
if path.is_dir() {
if let Ok(sub) = walkdir_depth(&path, max_depth - 1) {
results.extend(sub);
}
walkdir_depth_inner(&path, max_depth - 1, results, max);
} else {
results.push(path);
}
}
Ok(results)
}
/// Render text onto a transparent RGBA buffer and return it as a DynamicImage
@@ -333,7 +336,7 @@ fn apply_tiled_text_watermark(
let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8;
let draw_color = Rgba([color[0], color[1], color[2], alpha]);
let text_width = ((text.len().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as i64 + 4).min(8192);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as i64 + 4).min(8192);
let text_height = ((font_size.min(1000.0) * 1.4) as i64 + 4).min(4096);
let mut y = spacing as i64;
@@ -363,8 +366,9 @@ fn apply_tiled_image_watermark(
reason: format!("Failed to load watermark image: {}", e),
})?;
let wm_width = ((watermark.width() as f32 * scale) as u32).clamp(1, 16384);
let wm_height = ((watermark.height() as f32 * scale) as u32).clamp(1, 16384);
let safe_scale = if scale.is_finite() && scale > 0.0 { scale } else { 1.0 };
let wm_width = ((watermark.width() as f32 * safe_scale) as u32).clamp(1, 16384);
let wm_height = ((watermark.height() as f32 * safe_scale) as u32).clamp(1, 16384);
let mut watermark = watermark.resize_exact(wm_width, wm_height, image::imageops::FilterType::Lanczos3);
if let Some(rot) = rotation {
@@ -426,8 +430,9 @@ fn apply_image_watermark(
})?;
// Scale the watermark (capped to prevent OOM on extreme scale values)
let wm_width = ((watermark.width() as f32 * scale) as u32).clamp(1, 16384);
let wm_height = ((watermark.height() as f32 * scale) as u32).clamp(1, 16384);
let safe_scale = if scale.is_finite() && scale > 0.0 { scale } else { 1.0 };
let wm_width = ((watermark.width() as f32 * safe_scale) as u32).clamp(1, 16384);
let wm_height = ((watermark.height() as f32 * safe_scale) as u32).clamp(1, 16384);
let mut watermark = watermark.resize_exact(
wm_width,

2
pixstrip-core/src/storage.rs
View File

@@ -14,7 +14,7 @@ fn default_config_dir() -> PathBuf {
}
/// Write to a temporary file then rename, for crash safety.
fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> {
pub fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> {
let tmp = path.with_extension("tmp");
std::fs::write(&tmp, contents)?;
std::fs::rename(&tmp, path)?;

3
pixstrip-core/src/types.rs
View File

@@ -11,6 +11,7 @@ pub enum ImageFormat {
Avif,
Gif,
Tiff,
Bmp,
}
impl ImageFormat {
@@ -22,6 +23,7 @@ impl ImageFormat {
"avif" => Some(Self::Avif),
"gif" => Some(Self::Gif),
"tiff" | "tif" => Some(Self::Tiff),
"bmp" => Some(Self::Bmp),
_ => None,
}
}
@@ -34,6 +36,7 @@ impl ImageFormat {
Self::Avif => "avif",
Self::Gif => "gif",
Self::Tiff => "tiff",
Self::Bmp => "bmp",
}
}
}