Skip to content

Commit

Permalink
Check that data file is a child of user provided context dir.
Browse files Browse the repository at this point in the history
  • Loading branch information
jcamiel committed Feb 3, 2022
1 parent 77193af commit 0738a17
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 7 deletions.
22 changes: 15 additions & 7 deletions packages/hurl/src/runner/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}),
Expand Down
4 changes: 4 additions & 0 deletions packages/hurl/src/runner/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ pub enum RunnerError {
UnrenderableVariable {
value: String,
},

UnauthorizedFileAccess {
path: PathBuf,
},
}

// endregion
4 changes: 4 additions & 0 deletions packages/hurl/src/runner/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -138,6 +139,9 @@ impl Error for runner::Error {
RunnerError::InvalidJson { value } => {
format!("actual value is <{}>", value)
}
RunnerError::UnauthorizedFileAccess { path } => {
format!("Unauthorized access to file {}", path.to_str().unwrap())
}
}
}
}
1 change: 1 addition & 0 deletions packages/hurl/src/runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ mod http_response;
mod hurl_file;
mod json;
mod multipart;
mod path;
mod predicate;
mod predicate_value;
mod query;
Expand Down
96 changes: 96 additions & 0 deletions packages/hurl/src/runner/path.rs
Original file line number Diff line number Diff line change
@@ -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));
}
}

0 comments on commit 0738a17

Please sign in to comment.