Check that data file is a child of user provided context dir.

integration/tests/error_file_unauthorized.err

@@ -0,0 +1,7 @@
+error: Unauthorized file access
+  --> tests/error_file_unauthorized.hurl:2:6
+   |
+ 2 | file,/secret.txt;
+   |      ^^^^^^^^^^^ Unauthorized access to file /secret.txt, check --file-root option
+   |
+

integration/tests/error_file_unauthorized.exit

@@ -0,0 +1 @@
+3

integration/tests/error_file_unauthorized.html

@@ -0,0 +1,5 @@
+<pre><code class="language-hurl"><span class="hurl-entry"><span class="request"><span class="line"><span class="method">POST</span> <span class="url">http://localhost:8000/post-file</span></span>
+<span class="line">file,<span class="filename">/secret.txt</span>;</span>
+</span><span class="response"><span class="line"></span>
+<span class="line"><span class="version">HTTP/1.0</span> <span class="number">200</span></span>
+</span></span></code></pre>

integration/tests_error_parser/invalid_file.hurl → integration/tests/error_file_unauthorized.hurl

@@ -1,4 +1,4 @@
 POST http://localhost:8000/post-file
-file,/data.bin;
+file,/secret.txt;
 
-HTTP/1.0 200
+HTTP/1.0 200

packages/hurl/src/runner/body.rs

@@ -26,6 +26,7 @@ use super::json::eval_json_value;
 use super::template::eval_template;
 use super::value::Value;
 use crate::http;
+use crate::runner::path;
 
 pub fn eval_body(
     body: Body,
@@ -57,17 +58,24 @@ pub fn eval_bytes(
         Bytes::File(File { filename, .. }) => {
             let f = filename.value.as_str();
             let path = Path::new(f);
-            let absolute_filename = if path.is_absolute() {
-                filename.clone().value
-            } else {
-                context_dir.join(f).to_str().unwrap().to_string()
-            };
-            match std::fs::read(absolute_filename.clone()) {
+            let absolute_path = context_dir.join(path);
+            // In order not to leak any private date, we check that the user provided file
+            // is a child of the context directory.
+            if !path::is_descendant(&absolute_path, context_dir) {
+                return Err(Error {
+                    source_info: filename.source_info,
+                    inner: RunnerError::UnauthorizedFileAccess {
+                        path: absolute_path,
+                    },
+                    assert: false,
+                });
+            }
+            match std::fs::read(&absolute_path) {
                 Ok(value) => Ok(http::Body::File(value, f.to_string())),
                 Err(_) => Err(Error {
                     source_info: filename.source_info,
                     inner: RunnerError::FileReadAccess {
-                        value: absolute_filename,
+                        value: absolute_path.to_str().unwrap().to_string(),
                     },
                     assert: false,
                 }),

packages/hurl/src/runner/core.rs

@@ -190,6 +190,10 @@ pub enum RunnerError {
     UnrenderableVariable {
         value: String,
     },
+
+    UnauthorizedFileAccess {
+        path: PathBuf,
+    },
 }
 
 // endregion

packages/hurl/src/runner/error.rs

@@ -49,6 +49,7 @@ impl Error for runner::Error {
             RunnerError::UnsupportedContentEncoding(..) => "Decompression Error".to_string(),
             RunnerError::CouldNotUncompressResponse(..) => "Decompression Error".to_string(),
             RunnerError::InvalidJson { .. } => "Invalid Json".to_string(),
+            RunnerError::UnauthorizedFileAccess { .. } => "Unauthorized file access".to_string(),
         }
     }
 
@@ -138,6 +139,12 @@ impl Error for runner::Error {
             RunnerError::InvalidJson { value } => {
                 format!("actual value is <{}>", value)
             }
+            RunnerError::UnauthorizedFileAccess { path } => {
+                format!(
+                    "Unauthorized access to file {}, check --file-root option",
+                    path.to_str().unwrap()
+                )
+            }
         }
     }
 }

packages/hurl/src/runner/mod.rs

@@ -43,6 +43,7 @@ mod http_response;
 mod hurl_file;
 mod json;
 mod multipart;
+mod path;
 mod predicate;
 mod predicate_value;
 mod query;

packages/hurl/src/runner/path.rs

@@ -0,0 +1,96 @@
+/*
+ * Hurl (https://hurl.dev)
+ * Copyright (C) 2022 Orange
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *          http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+use std::path::{Component, Path, PathBuf};
+
+/// Return true if `path` is a descendant path of ancestor, false otherwise
+pub fn is_descendant(path: &Path, ancestor: &Path) -> bool {
+    let path = normalize_path(path);
+    let ancestor = normalize_path(ancestor);
+    for a in path.ancestors() {
+        if ancestor == a {
+            return true;
+        }
+    }
+    false
+}
+
+/// Returns the absolute form of the path with all intermediate components normalized.
+/// Contrary to the methods `canonicalize` on `Path`, this function doesn't require
+/// the final path to exist.
+/// Borrowed from https://github.com/rust-lang/cargo src/cargo/util/paths.rs
+fn normalize_path(path: &Path) -> PathBuf {
+    let mut components = path.components().peekable();
+    let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() {
+        components.next();
+        PathBuf::from(c.as_os_str())
+    } else {
+        PathBuf::new()
+    };
+
+    for component in components {
+        match component {
+            Component::Prefix(..) => unreachable!(),
+            Component::RootDir => {
+                ret.push(component.as_os_str());
+            }
+            Component::CurDir => {}
+            Component::ParentDir => {
+                ret.pop();
+            }
+            Component::Normal(c) => {
+                ret.push(c);
+            }
+        }
+    }
+    ret
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn is_descendant_true() {
+        let child = Path::new("/tmp/foo/bar.txt");
+        let parent = Path::new("/tmp");
+        assert!(is_descendant(child, parent));
+
+        let child = Path::new("/tmp/foo/../bar.txt");
+        let parent = Path::new("/tmp");
+        assert!(is_descendant(child, parent));
+
+        let child = Path::new("bar.txt");
+        let parent = Path::new("");
+        assert!(is_descendant(child, parent));
+    }
+
+    #[test]
+    fn is_descendant_false() {
+        let child = Path::new("/tmp/foo/../../bar.txt");
+        let parent = Path::new("/tmp");
+        assert!(!is_descendant(child, parent));
+
+        let child = Path::new("/a/bar.txt");
+        let parent = Path::new("/b");
+        assert!(!is_descendant(child, parent));
+
+        let child = Path::new("/bar.txt");
+        let parent = Path::new("");
+        assert!(!is_descendant(child, parent));
+    }
+}

packages/hurl_core/src/parser/filename.rs

@@ -23,7 +23,7 @@ use super::ParseResult;
 use crate::ast::*;
 
 pub fn parse(reader: &mut Reader) -> ParseResult<'static, Filename> {
-    // this is an absolute file
+    // This is an absolute file
     // that you have to write with a relative name
     // default root_dir is the hurl directory
     let start = reader.state.clone();
@@ -37,13 +37,6 @@ pub fn parse(reader: &mut Reader) -> ParseResult<'static, Filename> {
             inner: ParseError::Filename {},
         });
     }
-    if s.starts_with('/') {
-        return Err(Error {
-            pos: start.pos,
-            recoverable: false,
-            inner: ParseError::Filename {},
-        });
-    }
 
     Ok(Filename {
         value: s,
@@ -102,11 +95,5 @@ mod tests {
         let error = parse(&mut reader).err().unwrap();
         assert_eq!(error.inner, ParseError::Filename {});
         assert_eq!(error.pos, Pos { line: 1, column: 1 });
-
-        // can not absolute
-        let mut reader = Reader::init("/tmp/data.bin");
-        let error = parse(&mut reader).err().unwrap();
-        assert_eq!(error.inner, ParseError::Filename {});
-        assert_eq!(error.pos, Pos { line: 1, column: 1 });
     }
 }