From 45606f457999ff0f378fed52300364bb700fce51 Mon Sep 17 00:00:00 2001 From: "Aode (Lion)" Date: Thu, 9 Sep 2021 14:16:12 -0500 Subject: [PATCH] Add application-level dimension limiting, bail on failed processes --- src/config.rs | 24 ++++++++++++++++++ src/magick.rs | 67 +++++++++++++++++++-------------------------------- src/stream.rs | 43 +++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 42 deletions(-) diff --git a/src/config.rs b/src/config.rs index ac21557..71dc896 100644 --- a/src/config.rs +++ b/src/config.rs @@ -51,6 +51,22 @@ pub(crate) struct Config { )] max_file_size: usize, + #[structopt( + long, + env = "PICTRS_MAX_IMAGE_WIDTH", + help = "Specify the maximum width in pixels allowed on an image", + default_value = "10000" + )] + max_image_width: usize, + + #[structopt( + long, + env = "PICTRS_MAX_IMAGE_HEIGHT", + help = "Specify the maximum width in pixels allowed on an image", + default_value = "10000" + )] + max_image_height: usize, + #[structopt( long, env = "PICTRS_API_KEY", @@ -86,6 +102,14 @@ impl Config { self.max_file_size } + pub(crate) fn max_width(&self) -> usize { + self.max_image_width + } + + pub(crate) fn max_height(&self) -> usize { + self.max_image_height + } + pub(crate) fn api_key(&self) -> Option<&str> { self.api_key.as_deref() } diff --git a/src/magick.rs b/src/magick.rs index b968067..1684dc2 100644 --- a/src/magick.rs +++ b/src/magick.rs @@ -1,8 +1,7 @@ use crate::{config::Format, stream::Process}; use actix_web::web::Bytes; -use std::process::Stdio; use tokio::{ - io::{AsyncRead, AsyncReadExt, AsyncWriteExt}, + io::{AsyncRead, AsyncReadExt}, process::Command, }; @@ -13,6 +12,9 @@ pub(crate) enum MagickError { #[error("Invalid format")] Format, + + #[error("Image too large")] + Dimensions, } pub(crate) enum ValidInputType { @@ -124,46 +126,8 @@ fn parse_details(s: std::borrow::Cow<'_, str>) -> Result { }) } -pub(crate) async fn input_type_bytes(mut input: Bytes) -> Result { - let mut child = Command::new("magick") - .args(["identify", "-ping", "-format", "%m\n", "-"]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn()?; - - let mut stdin = child.stdin.take().unwrap(); - let mut stdout = child.stdout.take().unwrap(); - - stdin.write_all_buf(&mut input).await?; - drop(stdin); - - let mut vec = Vec::new(); - stdout.read_to_end(&mut vec).await?; - drop(stdout); - - child.wait().await?; - - let s = String::from_utf8_lossy(&vec); - parse_input_type(s) -} - -fn parse_input_type(s: std::borrow::Cow<'_, str>) -> Result { - let mut lines = s.lines(); - let first = lines.next(); - - let opt = lines.fold(first, |acc, item| match acc { - Some(prev) if prev == item => Some(prev), - _ => None, - }); - - match opt { - Some("MP4") => Ok(ValidInputType::Mp4), - Some("GIF") => Ok(ValidInputType::Gif), - Some("PNG") => Ok(ValidInputType::Png), - Some("JPEG") => Ok(ValidInputType::Jpeg), - Some("WEBP") => Ok(ValidInputType::Webp), - _ => Err(MagickError::Format), - } +pub(crate) async fn input_type_bytes(input: Bytes) -> Result { + details_bytes(input).await?.validate_input() } pub(crate) fn process_image_write_read( @@ -181,6 +145,25 @@ pub(crate) fn process_image_write_read( Ok(process.write_read(input).unwrap()) } +impl Details { + fn validate_input(&self) -> Result { + if self.width > crate::CONFIG.max_width() || self.height > crate::CONFIG.max_height() { + return Err(MagickError::Dimensions); + } + + let input_type = match (self.mime_type.type_(), self.mime_type.subtype()) { + (mime::VIDEO, mime::MP4 | mime::MPEG) => ValidInputType::Mp4, + (mime::IMAGE, mime::GIF) => ValidInputType::Gif, + (mime::IMAGE, mime::PNG) => ValidInputType::Png, + (mime::IMAGE, mime::JPEG) => ValidInputType::Jpeg, + (mime::IMAGE, subtype) if subtype.as_str() == "webp" => ValidInputType::Webp, + _ => return Err(MagickError::Format), + }; + + Ok(input_type) + } +} + impl From for MagickError { fn from(_: std::num::ParseIntError) -> MagickError { MagickError::Format diff --git a/src/stream.rs b/src/stream.rs index de43ae4..5d88e60 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -8,6 +8,9 @@ use std::{ }; use tokio::io::{AsyncRead, AsyncWriteExt, ReadBuf}; +#[derive(Debug)] +struct StatusError; + pub(crate) struct Process { child: tokio::process::Child, } @@ -38,9 +41,25 @@ impl Process { let (tx, rx) = tokio::sync::oneshot::channel(); + let mut child = self.child; + actix_rt::spawn(async move { if let Err(e) = stdin.write_all_buf(&mut input).await { let _ = tx.send(e); + return; + } + drop(stdin); + + match child.wait().await { + Ok(status) => { + if !status.success() { + let _ = + tx.send(std::io::Error::new(std::io::ErrorKind::Other, &StatusError)); + } + } + Err(e) => { + let _ = tx.send(e); + } } }); @@ -60,9 +79,25 @@ impl Process { let (tx, rx) = tokio::sync::oneshot::channel(); + let mut child = self.child; + actix_rt::spawn(async move { if let Err(e) = tokio::io::copy(&mut input_reader, &mut stdin).await { let _ = tx.send(e); + return; + } + drop(stdin); + + match child.wait().await { + Ok(status) => { + if !status.success() { + let _ = + tx.send(std::io::Error::new(std::io::ErrorKind::Other, &StatusError)); + } + } + Err(e) => { + let _ = tx.send(e); + } } }); @@ -122,3 +157,11 @@ where .map_err(UploadError::from) } } + +impl std::fmt::Display for StatusError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "Command failed with bad status") + } +} + +impl std::error::Error for StatusError {}