From 96eadb17232b50d382ebeda3532949370428c93e Mon Sep 17 00:00:00 2001 From: neri Date: Thu, 18 Aug 2022 23:20:56 +0200 Subject: [PATCH] return correct mime types, improve web security --- init-db.sql | 2 ++ src/deleter.rs | 2 +- src/download.rs | 66 +++++++++++++++++++++++++++++------------------- src/file_kind.rs | 27 -------------------- src/main.rs | 10 ++++++-- src/multipart.rs | 55 +++++++++++++++++++++++----------------- src/upload.rs | 24 ++++++++++-------- 7 files changed, 96 insertions(+), 90 deletions(-) delete mode 100644 src/file_kind.rs diff --git a/init-db.sql b/init-db.sql index 5f2ab73..2ae5e82 100644 --- a/init-db.sql +++ b/init-db.sql @@ -10,3 +10,5 @@ CREATE TABLE IF NOT EXISTS files ( ALTER TABLE files ADD COLUMN IF NOT EXISTS delete_on_download boolean; ALTER TABLE files ALTER COLUMN delete_on_download set not null; ALTER TABLE files ALTER COLUMN valid_till TYPE timestamptz; +ALTER TABLE files DROP COLUMN IF EXISTS kind; +ALTER TABLE files ADD COLUMN IF NOT EXISTS content_type varchar(255); diff --git a/src/deleter.rs b/src/deleter.rs index 7a77ef1..f7e6f6a 100644 --- a/src/deleter.rs +++ b/src/deleter.rs @@ -1,9 +1,9 @@ use futures_util::TryStreamExt; -use time::OffsetDateTime; use sqlx::{postgres::PgPool, Row}; use std::cmp::max; use std::path::{Path, PathBuf}; use time::ext::NumericalStdDuration; +use time::OffsetDateTime; use tokio::fs; use tokio::sync::mpsc::Receiver; use tokio::time::timeout; diff --git a/src/download.rs b/src/download.rs index b7cc1ad..b8801d1 100644 --- a/src/download.rs +++ b/src/download.rs @@ -6,19 +6,19 @@ use actix_web::{ http::header::{ Accept, CacheControl, CacheDirective, Charset, ContentDisposition, DispositionParam, DispositionType, Expires, ExtendedValue, Header, HeaderValue, HttpDate, TryIntoHeaderValue, - ACCEPT, CACHE_CONTROL, EXPIRES, VARY, + ACCEPT, CACHE_CONTROL, CONTENT_TYPE, EXPIRES, VARY, X_CONTENT_TYPE_OPTIONS, }, web, Error, HttpRequest, HttpResponse, }; -use mime::{Mime, TEXT_HTML}; +use mime::{Mime, APPLICATION_OCTET_STREAM, TEXT_HTML}; use sqlx::postgres::PgPool; use std::path::Path; use time::OffsetDateTime; use tokio::fs; use url::Url; +use crate::config::Config; use crate::deleter; -use crate::{config::Config, file_kind::FileKind}; const TEXT_VIEW_HTML: &str = include_str!("../template/text-view.html"); const URL_VIEW_HTML: &str = include_str!("../template/url-view.html"); @@ -37,14 +37,14 @@ pub async fn download( config: web::Data, ) -> Result { let id = req.match_info().query("id"); - let (file_id, file_name, valid_till, file_kind, delete) = load_file_info(id, &db).await?; + let (file_id, file_name, valid_till, content_type, delete) = load_file_info(id, &db).await?; let mut path = config.files_dir.clone(); path.push(&file_id); - let file_mime = get_content_type(&path); - let mut response = match get_view_type(&req, &file_kind, &file_mime, &path, delete).await { - ViewType::Raw => build_file_response(false, &file_name, path, file_mime, &req).await, - ViewType::Download => build_file_response(true, &file_name, path, file_mime, &req).await, + let mime = Mime::from_str(&content_type).unwrap_or(APPLICATION_OCTET_STREAM); + let mut response = match get_view_type(&req, &mime, &path, delete).await { + ViewType::Raw => build_file_response(false, &file_name, path, mime, &req).await, + ViewType::Download => build_file_response(true, &file_name, path, mime, &req).await, ViewType::Html => build_text_response(&path).await, }?; @@ -67,7 +67,7 @@ async fn load_file_info( db: &web::Data>, ) -> Result<(String, String, OffsetDateTime, String, bool), Error> { sqlx::query_as( - "SELECT file_id, file_name, valid_till, kind, delete_on_download from files WHERE file_id = $1", + "SELECT file_id, file_name, valid_till, content_type, delete_on_download from files WHERE file_id = $1", ) .bind(id) .fetch_optional(db.as_ref()) @@ -79,18 +79,9 @@ async fn load_file_info( .ok_or_else(|| error::ErrorNotFound("file does not exist or has expired")) } -fn get_content_type(path: &Path) -> Mime { - let std_path = std::path::Path::new(path.as_os_str()); - tree_magic_mini::from_filepath(std_path) - .unwrap_or("application/octet-stream") - .parse::() - .expect("tree_magic_mini should not produce invalid mime") -} - async fn get_view_type( req: &HttpRequest, - file_kind: &str, - file_mime: &Mime, + mime: &Mime, file_path: &Path, delete_on_download: bool, ) -> ViewType { @@ -100,9 +91,7 @@ async fn get_view_type( if req.query_string().contains("raw") { return ViewType::Raw; } - let is_text = - FileKind::from_str(file_kind) == Ok(FileKind::Text) || file_mime.type_() == mime::TEXT; - if !is_text { + if mime.type_() != mime::TEXT { return ViewType::Raw; } if get_file_size(file_path).await >= TEXT_VIEW_SIZE_LIMIT { @@ -113,7 +102,7 @@ async fn get_view_type( if accept_mime == TEXT_HTML { return ViewType::Html; } - if mime_matches(&accept_mime, file_mime) { + if mime_matches(&accept_mime, mime) { break; } } @@ -157,7 +146,7 @@ async fn build_file_response( download: bool, file_name: &str, path: PathBuf, - content_type: Mime, + mime: Mime, req: &HttpRequest, ) -> Result { let content_disposition = ContentDisposition { @@ -173,10 +162,35 @@ async fn build_file_response( log::error!("file could not be read {:?}", file_err); error::ErrorInternalServerError("this file should be here but could not be found") })? - .set_content_type(content_type) + .set_content_type(mime) .set_content_disposition(content_disposition); - Ok(file.into_response(req)) + let mut response = file.into_response(req); + add_headers(req, download, &mut response); + Ok(response) +} + +fn add_headers(req: &HttpRequest, download: bool, response: &mut HttpResponse) { + // if the browser is trying to fetch this resource in a secure context pretend the reponse is + // just binary data so it won't be executed + let sec_fetch_mode = req + .headers() + .get("sec-fetch-mode") + .and_then(|v| v.to_str().ok()); + if !download && sec_fetch_mode.is_some() && sec_fetch_mode != Some("navigate") { + response.headers_mut().insert( + CONTENT_TYPE, + HeaderValue::from_str(APPLICATION_OCTET_STREAM.as_ref()) + .expect("mime type can be encoded to header value"), + ); + response + .headers_mut() + .insert(X_CONTENT_TYPE_OPTIONS, HeaderValue::from_static("nosniff")); + } + // the reponse varies based on these request headers + response + .headers_mut() + .append(VARY, HeaderValue::from_static("accept, sec-fetch-mode")); } fn get_disposition_params(filename: &str) -> Vec { diff --git a/src/file_kind.rs b/src/file_kind.rs deleted file mode 100644 index 4b3b4f2..0000000 --- a/src/file_kind.rs +++ /dev/null @@ -1,27 +0,0 @@ -use std::{fmt::Display, str::FromStr}; - -#[derive(Debug, PartialEq)] -pub(crate) enum FileKind { - Text, - Binary, -} - -impl Display for FileKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - FileKind::Text => write!(f, "text"), - FileKind::Binary => write!(f, "binary"), - } - } -} - -impl FromStr for FileKind { - type Err = String; - fn from_str(s: &str) -> Result { - match s.to_lowercase().as_str() { - "text" => Ok(FileKind::Text), - "binary" => Ok(FileKind::Binary), - _ => Err(format!("unknown kind {}", s)), - } - } -} diff --git a/src/main.rs b/src/main.rs index 0778f24..41f07a2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,14 +2,14 @@ mod config; mod db; mod deleter; mod download; -mod file_kind; mod multipart; mod template; mod upload; use actix_files::Files; use actix_web::{ - middleware::{self, Logger}, + http::header::{HeaderName, CONTENT_SECURITY_POLICY}, + middleware::{self, DefaultHeaders, Logger}, web::{self, Data}, App, Error, HttpResponse, HttpServer, }; @@ -18,6 +18,11 @@ use sqlx::postgres::PgPool; use std::env; use tokio::{sync::mpsc::channel, task}; +const DEFAULT_CSP: (HeaderName, &str) = ( + CONTENT_SECURITY_POLICY, + "default-src 'none'; connect-src 'self'; img-src 'self'; media-src 'self'; font-src 'self'; script-src 'self'; style-src 'self'; object-src 'none'; base-uri 'self'; frame-src 'none'; frame-ancestors 'none'; form-action 'self';" +); + async fn not_found() -> Result { Ok(HttpResponse::NotFound() .content_type("text/plain") @@ -51,6 +56,7 @@ async fn main() -> std::io::Result<()> { move || { App::new() .wrap(Logger::new(r#"%{r}a "%r" =%s %bbytes %Tsec"#)) + .wrap(DefaultHeaders::new().add(DEFAULT_CSP)) .wrap(middleware::Compress::default()) .app_data(db.clone()) .app_data(expiry_watch_sender.clone()) diff --git a/src/multipart.rs b/src/multipart.rs index 2615c4c..9adfdac 100644 --- a/src/multipart.rs +++ b/src/multipart.rs @@ -1,7 +1,8 @@ -use crate::{config, file_kind::FileKind}; +use crate::config; use actix_multipart::{Field, Multipart}; use actix_web::{error, http::header::DispositionParam, Error}; use futures_util::{StreamExt, TryStreamExt}; +use mime::{Mime, TEXT_PLAIN}; use std::path::Path; use time::{Duration, OffsetDateTime}; use tokio::{fs::File, io::AsyncWriteExt}; @@ -10,21 +11,20 @@ const MAX_UPLOAD_SECONDS: i64 = 31 * 24 * 60 * 60; const DEFAULT_UPLOAD_SECONDS: u32 = 30 * 60; pub(crate) struct UploadConfig { - pub original_name: String, + pub original_name: Option, + pub content_type: Mime, pub valid_till: OffsetDateTime, - pub kind: FileKind, pub delete_on_download: bool, } pub(crate) async fn parse_multipart( mut payload: Multipart, - file_id: &str, - file_name: &Path, + file_path: &Path, config: &config::Config, ) -> Result { let mut original_name: Option = None; + let mut content_type: Option = None; let mut keep_for: Option = None; - let mut kind: Option = None; let mut delete_on_download = false; let mut password = None; let mut size = 0; @@ -37,22 +37,20 @@ pub(crate) async fn parse_multipart( keep_for = Some(parse_string(name, field).await?); } "file" => { - let file_original_name = get_original_filename(&field); - if file_original_name == None || file_original_name.map(|f| f.as_str()) == Some("") - { + let (mime, uploaded_name) = get_file_metadata(&field); + if uploaded_name == None || uploaded_name.map(|f| f.as_str()) == Some("") { continue; } - original_name = file_original_name.map(|f| f.to_string()); - kind = Some(FileKind::Binary); - size = create_file(file_name, field, config.max_file_size).await?; + original_name = uploaded_name.map(|f| f.to_string()); + content_type = Some(mime.clone()); + size = create_file(file_path, field, config.max_file_size).await?; } "text" => { if original_name.is_some() { continue; } - original_name = Some(format!("{}.txt", file_id)); - kind = Some(FileKind::Text); - size = create_file(file_name, field, config.max_file_size).await?; + size = create_file(file_path, field, config.max_file_size).await?; + content_type = Some(get_content_type(file_path)); } "delete_on_download" => { delete_on_download = parse_string(name, field).await? != "false"; @@ -64,8 +62,8 @@ pub(crate) async fn parse_multipart( }; } - let original_name = original_name.ok_or_else(|| error::ErrorBadRequest("no content found"))?; - let kind = kind.ok_or_else(|| error::ErrorBadRequest("no content found"))?; + let content_type = + content_type.ok_or_else(|| error::ErrorBadRequest("no content type found"))?; let keep_for: u32 = keep_for .map(|k| k.parse()) .transpose() @@ -76,8 +74,8 @@ pub(crate) async fn parse_multipart( let upload_config = UploadConfig { original_name, + content_type, valid_till, - kind, delete_on_download, }; @@ -93,8 +91,10 @@ fn check_requirements( valid_duration: &Duration, config: &config::Config, ) -> Result<(), error::Error> { - if upload_config.original_name.len() > 255 { - return Err(error::ErrorBadRequest("filename is too long")); + if let Some(original_name) = upload_config.original_name.as_ref() { + if original_name.len() > 255 { + return Err(error::ErrorBadRequest("filename is too long")); + } } let valid_seconds = valid_duration.whole_seconds(); @@ -180,13 +180,22 @@ async fn write_to_file( Ok(written_bytes) } -fn get_original_filename(field: &actix_multipart::Field) -> Option<&String> { - field +fn get_file_metadata(field: &actix_multipart::Field) -> (&Mime, Option<&String>) { + let mime = field.content_type(); + let filename = field .content_disposition() .parameters .iter() .find_map(|param| match param { DispositionParam::Filename(filename) => Some(filename), _ => None, - }) + }); + (mime, filename) +} + +fn get_content_type(path: &Path) -> Mime { + let std_path = std::path::Path::new(path.as_os_str()); + tree_magic_mini::from_filepath(std_path) + .and_then(|mime| mime.parse().ok()) + .unwrap_or(TEXT_PLAIN) } diff --git a/src/upload.rs b/src/upload.rs index 0c1a389..5878359 100644 --- a/src/upload.rs +++ b/src/upload.rs @@ -1,7 +1,6 @@ use std::io::ErrorKind; use crate::config::Config; -use crate::file_kind::FileKind; use crate::multipart::UploadConfig; use crate::{multipart, template}; use actix_files::NamedFile; @@ -42,11 +41,11 @@ pub async fn upload( error::ErrorInternalServerError("could not create file") })?; - let parsed_multipart = multipart::parse_multipart(payload, &file_id, &file_name, &config).await; + let parsed_multipart = multipart::parse_multipart(payload, &file_name, &config).await; let UploadConfig { original_name, + content_type, valid_till, - kind, delete_on_download, } = match parsed_multipart { Ok(data) => data, @@ -65,14 +64,17 @@ pub async fn upload( } }; + let file_name = original_name + .clone() + .unwrap_or_else(|| format!("{}.txt", file_id)); let db_insert = sqlx::query( - "INSERT INTO Files (file_id, file_name, valid_till, kind, delete_on_download) \ + "INSERT INTO Files (file_id, file_name, content_type, valid_till, delete_on_download) \ VALUES ($1, $2, $3, $4, $5)", ) .bind(&file_id) - .bind(&original_name) + .bind(&file_name) + .bind(&content_type.to_string()) .bind(valid_till) - .bind(kind.to_string()) .bind(delete_on_download) .execute(db.as_ref()) .await; @@ -90,24 +92,24 @@ pub async fn upload( } log::info!( - "{} create new file {} (valid_till: {}, kind: {}, delete_on_download: {})", + "{} create new file {} (valid_till: {}, content_type: {}, delete_on_download: {})", req.connection_info().realip_remote_addr().unwrap_or("-"), file_id, valid_till, - kind, + content_type, delete_on_download ); expiry_watch_sender.send(()).await.unwrap(); - let redirect = if kind == FileKind::Binary { - let encoded_name = urlencoding::encode(&original_name); + let redirect = if let Some(original_name) = original_name.as_ref() { + let encoded_name = urlencoding::encode(original_name); format!("/upload/{}/{}", file_id, encoded_name) } else { format!("/upload/{}", file_id) }; - let url = get_file_url(&req, &file_id, Some(&original_name)); + let url = get_file_url(&req, &file_id, original_name.as_deref()); Ok(HttpResponse::SeeOther() .insert_header((LOCATION, redirect)) .body(format!("{}\n", url)))