Browse Source

Protect namedfile against path traversal, rocket only does it for pathbuf

Daniel García 4 years ago
parent
commit
e5ec245626
3 changed files with 36 additions and 4 deletions
  1. 2 1
      src/api/core/sends.rs
  2. 2 2
      src/api/web.rs
  3. 32 1
      src/util.rs

+ 2 - 1
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();

+ 2 - 2
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()
 }
 

+ 32 - 1
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] =