From e5ec245626e4a4d232b6a709338fe1e9811845e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa?=
 <dani-garcia@users.noreply.github.com>
Date: Thu, 15 Jul 2021 19:15:55 +0200
Subject: [PATCH] Protect namedfile against path traversal, rocket only does it
 for pathbuf

---
 src/api/core/sends.rs |  3 ++-
 src/api/web.rs        |  4 ++--
 src/util.rs           | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs
index 13cd300e..ab0c7a36 100644
--- a/src/api/core/sends.rs
+++ b/src/api/core/sends.rs
@@ -10,6 +10,7 @@ use crate::{
     api::{ApiResult, EmptyResult, JsonResult, JsonUpcase, Notify, UpdateType},
     auth::{Headers, Host},
     db::{models::*, DbConn, DbPool},
+    util::SafeString,
     CONFIG,
 };
 
@@ -335,7 +336,7 @@ fn post_access_file(
 }
 
 #[get("/sends/<send_id>/<file_id>?<t>")]
-fn download_send(send_id: String, file_id: String, t: String) -> Option<NamedFile> {
+fn download_send(send_id: SafeString, file_id: SafeString, t: String) -> Option<NamedFile> {
     if let Ok(claims) = crate::auth::decode_send(&t) {
         if claims.sub == format!("{}/{}", send_id, file_id) {
             return NamedFile::open(Path::new(&CONFIG.sends_folder()).join(send_id).join(file_id)).ok();
diff --git a/src/api/web.rs b/src/api/web.rs
index 37950f0c..e543bc00 100644
--- a/src/api/web.rs
+++ b/src/api/web.rs
@@ -4,7 +4,7 @@ use rocket::{http::ContentType, response::content::Content, response::NamedFile,
 use rocket_contrib::json::Json;
 use serde_json::Value;
 
-use crate::{error::Error, util::Cached, CONFIG};
+use crate::{CONFIG, error::Error, util::{Cached, SafeString}};
 
 pub fn routes() -> Vec<Route> {
     // If addding more routes here, consider also adding them to
@@ -56,7 +56,7 @@ fn web_files(p: PathBuf) -> Cached<Option<NamedFile>> {
 }
 
 #[get("/attachments/<uuid>/<file_id>")]
-fn attachments(uuid: String, file_id: String) -> Option<NamedFile> {
+fn attachments(uuid: SafeString, file_id: SafeString) -> Option<NamedFile> {
     NamedFile::open(Path::new(&CONFIG.attachments_folder()).join(uuid).join(file_id)).ok()
 }
 
diff --git a/src/util.rs b/src/util.rs
index 8512bc7b..9e216d72 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -5,7 +5,8 @@ use std::io::Cursor;
 
 use rocket::{
     fairing::{Fairing, Info, Kind},
-    http::{ContentType, Header, HeaderMap, Method, Status},
+    http::{ContentType, Header, HeaderMap, Method, RawStr, Status},
+    request::FromParam,
     response::{self, Responder},
     Data, Request, Response, Rocket,
 };
@@ -125,6 +126,36 @@ impl<'r, R: Responder<'r>> Responder<'r> for Cached<R> {
     }
 }
 
+pub struct SafeString(String);
+
+impl std::fmt::Display for SafeString {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl AsRef<Path> for SafeString {
+    #[inline]
+    fn as_ref(&self) -> &Path {
+        Path::new(&self.0)
+    }
+}
+
+impl<'r> FromParam<'r> for SafeString {
+    type Error = ();
+
+    #[inline(always)]
+    fn from_param(param: &'r RawStr) -> Result<Self, Self::Error> {
+        let s = param.percent_decode().map(|cow| cow.into_owned()).map_err(|_| ())?;
+
+        if s.chars().all(|c| matches!(c, 'a'..='z' | 'A'..='Z' |'0'..='9' | '-')) {
+            Ok(SafeString(s))
+        } else {
+            Err(())
+        }
+    }
+}
+
 // Log all the routes from the main paths list, and the attachments endpoint
 // Effectively ignores, any static file route, and the alive endpoint
 const LOGGED_ROUTES: [&str; 6] =