public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox] rest: remove full static file path from error messages
Date: Tue, 20 Jun 2023 12:39:45 +0200	[thread overview]
Message-ID: <20230620103945.3969702-1-f.gruenbichler@proxmox.com> (raw)

this triggers certain security scanners, and having the requested path instead
gives basically the same information anyhow.

reported on the forum: https://forum.proxmox.com/threads/404-path-disclosure-vulnerability.129187/

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
with EPERM for example, this would return

 File open failed for 'foobar': permission denied

instead of

 File open failed: Permission denied (os error 13)

and for missing files it would return

 no such file: 'foobar'

instead of

 no such file: "/usr/share/javascript/proxmox-backup/foobar"

 proxmox-rest-server/src/rest.rs | 39 +++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/proxmox-rest-server/src/rest.rs b/proxmox-rest-server/src/rest.rs
index 100c93c..d22a35e 100644
--- a/proxmox-rest-server/src/rest.rs
+++ b/proxmox-rest-server/src/rest.rs
@@ -620,16 +620,12 @@ fn extension_to_content_type(filename: &Path) -> (&'static str, bool) {
 }
 
 async fn simple_static_file_download(
-    filename: PathBuf,
+    mut file: File,
     content_type: &'static str,
     compression: Option<CompressionMethod>,
 ) -> Result<Response<Body>, Error> {
     use tokio::io::AsyncReadExt;
 
-    let mut file = File::open(filename)
-        .await
-        .map_err(|err| http_err!(BAD_REQUEST, "File open failed: {}", err))?;
-
     let mut data: Vec<u8> = Vec::new();
 
     let mut response = match compression {
@@ -660,8 +656,8 @@ async fn simple_static_file_download(
     Ok(response)
 }
 
-async fn chuncked_static_file_download(
-    filename: PathBuf,
+async fn chunked_static_file_download(
+    file: File,
     content_type: &'static str,
     compression: Option<CompressionMethod>,
 ) -> Result<Response<Body>, Error> {
@@ -669,10 +665,6 @@ async fn chuncked_static_file_download(
         .status(StatusCode::OK)
         .header(header::CONTENT_TYPE, content_type);
 
-    let file = File::open(filename)
-        .await
-        .map_err(|err| http_err!(BAD_REQUEST, "File open failed: {}", err))?;
-
     let body = match compression {
         Some(CompressionMethod::Deflate) => {
             resp = resp.header(
@@ -691,24 +683,39 @@ async fn chuncked_static_file_download(
 }
 
 async fn handle_static_file_download(
+    components: &[&str],
     filename: PathBuf,
     compression: Option<CompressionMethod>,
 ) -> Result<Response<Body>, Error> {
     let metadata = match tokio::fs::metadata(filename.clone()).await {
         Ok(metadata) => metadata,
         Err(err) if err.kind() == io::ErrorKind::NotFound => {
-            http_bail!(NOT_FOUND, "no such file: {filename:?}")
+            http_bail!(NOT_FOUND, "no such file: '{}'", components.join("/"))
         }
-        Err(err) => http_bail!(BAD_REQUEST, "File access problems: {}", err),
+        Err(err) => http_bail!(
+            BAD_REQUEST,
+            "File access problem on '{}': {}",
+            components.join("/"),
+            err.kind()
+        ),
     };
 
     let (content_type, nocomp) = extension_to_content_type(&filename);
     let compression = if nocomp { None } else { compression };
 
+    let file = File::open(filename).await.map_err(|err| {
+        http_err!(
+            BAD_REQUEST,
+            "File open failed for '{}': {}",
+            components.join("/"),
+            err.kind()
+        )
+    })?;
+
     if metadata.len() < CHUNK_SIZE_LIMIT {
-        simple_static_file_download(filename, content_type, compression).await
+        simple_static_file_download(file, content_type, compression).await
     } else {
-        chuncked_static_file_download(filename, content_type, compression).await
+        chunked_static_file_download(file, content_type, compression).await
     }
 }
 
@@ -782,7 +789,7 @@ impl ApiConfig {
         } else {
             let filename = self.find_alias(&components);
             let compression = extract_compression_method(&parts.headers);
-            return handle_static_file_download(filename, compression).await;
+            return handle_static_file_download(&components, filename, compression).await;
         }
     }
 }
-- 
2.39.2





             reply	other threads:[~2023-06-20 10:39 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-20 10:39 Fabian Grünbichler [this message]
2023-06-23  9:48 ` [pbs-devel] applied: " Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230620103945.3969702-1-f.gruenbichler@proxmox.com \
    --to=f.gruenbichler@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal