Apps/ImageApi
1
0
Fork 0
Code Issues Pull Requests 1 Releases Wiki Activity

Fix file upload

Browse Source 
Add a flag for new files so we can skip the exists check when seeing if
the new file is within the base directory.
This commit is contained in:
Cameron Cordes
2023-04-03 08:41:03 -04:00
parent 7863990c68
commit 8bcd837038
2 changed files with 20 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
src/files.rs
View File

@@ -137,6 +137,7 @@ pub fn is_image_or_video(path: &Path) -> bool {
pub fn is_valid_full_path<P: AsRef<Path> + Debug + AsRef<std::ffi::OsStr>>( pub fn is_valid_full_path<P: AsRef<Path> + Debug + AsRef<std::ffi::OsStr>>(
base: &P, base: &P,
path: &P, path: &P,
new_file: bool,
) -> Option<PathBuf> { ) -> Option<PathBuf> {
debug!("Base: {:?}. Path: {:?}", base, path); debug!("Base: {:?}. Path: {:?}", base, path);
@@ -150,7 +151,7 @@ pub fn is_valid_full_path<P: AsRef<Path> + Debug + AsRef<std::ffi::OsStr>>(
path path
}; };
match is_path_above_base_dir(base, &mut path) { match is_path_above_base_dir(base, &mut path, new_file) {
Ok(path) => Some(path), Ok(path) => Some(path),
Err(e) => { Err(e) => {
error!("{}", e); error!("{}", e);
@@ -162,6 +163,7 @@ pub fn is_valid_full_path<P: AsRef<Path> + Debug + AsRef<std::ffi::OsStr>>(
fn is_path_above_base_dir<P: AsRef<Path> + Debug>( fn is_path_above_base_dir<P: AsRef<Path> + Debug>(
base: P, base: P,
full_path: &mut PathBuf, full_path: &mut PathBuf,
new_file: bool,
) -> anyhow::Result<PathBuf> { ) -> anyhow::Result<PathBuf> {
full_path full_path
.absolutize() .absolutize()
@@ -169,7 +171,7 @@ fn is_path_above_base_dir<P: AsRef<Path> + Debug>(
.map_or_else( .map_or_else(
|e| Err(anyhow!(e)), |e| Err(anyhow!(e)),
|p| { |p| {
if p.starts_with(base) && p.exists() { if p.starts_with(base) && (new_file || p.exists()) {
Ok(p.into_owned()) Ok(p.into_owned())
} else if !p.exists() { } else if !p.exists() {
Err(anyhow!("Path does not exist: {:?}", p)) Err(anyhow!("Path does not exist: {:?}", p))
@@ -196,7 +198,7 @@ impl RealFileSystem {
impl FileSystemAccess for RealFileSystem { impl FileSystemAccess for RealFileSystem {
fn get_files_for_path(&self, path: &str) -> anyhow::Result<Vec<PathBuf>> { fn get_files_for_path(&self, path: &str) -> anyhow::Result<Vec<PathBuf>> {
is_valid_full_path(&PathBuf::from(&self.base_path), &PathBuf::from(path)) is_valid_full_path(&PathBuf::from(&self.base_path), &PathBuf::from(path), false)
.map(|path| { .map(|path| {
debug!("Valid path: {:?}", path); debug!("Valid path: {:?}", path);
list_files(&path).unwrap_or_default() list_files(&path).unwrap_or_default()
@@ -457,23 +459,23 @@ mod tests {
#[test] #[test]
fn directory_traversal_test() { fn directory_traversal_test() {
let base = env::temp_dir(); let base = env::temp_dir();
assert_eq!(None, is_valid_full_path(&base, &PathBuf::from("../"))); assert_eq!(None, is_valid_full_path(&base, &PathBuf::from("../"), false));
assert_eq!(None, is_valid_full_path(&base, &PathBuf::from(".."))); assert_eq!(None, is_valid_full_path(&base, &PathBuf::from(".."), false));
assert_eq!( assert_eq!(
None, None,
is_valid_full_path(&base, &PathBuf::from("fake/../../../")) is_valid_full_path(&base, &PathBuf::from("fake/../../../"), false)
); );
assert_eq!( assert_eq!(
None, None,
is_valid_full_path(&base, &PathBuf::from("../../../etc/passwd")) is_valid_full_path(&base, &PathBuf::from("../../../etc/passwd"), false)
); );
assert_eq!( assert_eq!(
None, None,
is_valid_full_path(&base, &PathBuf::from("..//etc/passwd")) is_valid_full_path(&base, &PathBuf::from("..//etc/passwd"), false)
); );
assert_eq!( assert_eq!(
None, None,
is_valid_full_path(&base, &PathBuf::from("../../etc/passwd")) is_valid_full_path(&base, &PathBuf::from("../../etc/passwd"), false)
); );
} }
@@ -484,7 +486,7 @@ mod tests {
test_file.push("test.png"); test_file.push("test.png");
File::create(test_file).unwrap(); File::create(test_file).unwrap();
assert!(is_valid_full_path(&base, &PathBuf::from("test.png")).is_some()); assert!(is_valid_full_path(&base, &PathBuf::from("test.png"), false).is_some());
} }
#[test] #[test]
@@ -495,7 +497,7 @@ mod tests {
let mut test_file = PathBuf::from(&base); let mut test_file = PathBuf::from(&base);
test_file.push(path); test_file.push(path);
assert_eq!(None, is_valid_full_path(&base, &test_file)); assert_eq!(None, is_valid_full_path(&base, &test_file, false));
} }
#[test] #[test]
@@ -505,11 +507,11 @@ mod tests {
test_file.push("test.png"); test_file.push("test.png");
File::create(&test_file).unwrap(); File::create(&test_file).unwrap();
assert!(is_valid_full_path(&base, &test_file).is_some()); assert!(is_valid_full_path(&base, &test_file, false).is_some());
assert_eq!( assert_eq!(
Some(PathBuf::from("/tmp/test.png")), Some(PathBuf::from("/tmp/test.png")),
is_valid_full_path(&base, &PathBuf::from("/tmp/test.png")) is_valid_full_path(&base, &PathBuf::from("/tmp/test.png"), false)
); );
} }

9
src/main.rs
View File

@@ -76,7 +76,7 @@ async fn get_image(
req: web::Query<ThumbnailRequest>, req: web::Query<ThumbnailRequest>,
app_state: Data<AppState>, app_state: Data<AppState>,
) -> impl Responder { ) -> impl Responder {
if let Some(path) = is_valid_full_path(&app_state.base_path, &req.path) { if let Some(path) = is_valid_full_path(&app_state.base_path, &req.path, false) {
if req.size.is_some() { if req.size.is_some() {
let relative_path = path let relative_path = path
.strip_prefix(&app_state.base_path) .strip_prefix(&app_state.base_path)
@@ -108,7 +108,7 @@ async fn get_file_metadata(
path: web::Query<ThumbnailRequest>, path: web::Query<ThumbnailRequest>,
app_state: Data<AppState>, app_state: Data<AppState>,
) -> impl Responder { ) -> impl Responder {
match is_valid_full_path(&app_state.base_path, &path.path) match is_valid_full_path(&app_state.base_path, &path.path, false)
.ok_or_else(|| ErrorKind::InvalidData.into()) .ok_or_else(|| ErrorKind::InvalidData.into())
.and_then(File::open) .and_then(File::open)
.and_then(|file| file.metadata()) .and_then(|file| file.metadata())
@@ -159,6 +159,7 @@ async fn upload_image(
if let Some(full_path) = is_valid_full_path( if let Some(full_path) = is_valid_full_path(
&app_state.base_path, &app_state.base_path,
&full_path.to_str().unwrap().to_string(), &full_path.to_str().unwrap().to_string(),
true,
) { ) {
if !full_path.is_file() && is_image_or_video(&full_path) { if !full_path.is_file() && is_image_or_video(&full_path) {
let mut file = File::create(full_path).unwrap(); let mut file = File::create(full_path).unwrap();
@@ -188,7 +189,7 @@ async fn generate_video(
if let Some(name) = filename.file_stem() { if let Some(name) = filename.file_stem() {
let filename = name.to_str().expect("Filename should convert to string"); let filename = name.to_str().expect("Filename should convert to string");
let playlist = format!("tmp/{}.m3u8", filename); let playlist = format!("tmp/{}.m3u8", filename);
if let Some(path) = is_valid_full_path(&app_state.base_path, &body.path) { if let Some(path) = is_valid_full_path(&app_state.base_path, &body.path, false) {
if let Ok(child) = create_playlist(path.to_str().unwrap(), &playlist).await { if let Ok(child) = create_playlist(path.to_str().unwrap(), &playlist).await {
app_state app_state
.stream_manager .stream_manager
@@ -216,7 +217,7 @@ async fn stream_video(
debug!("Playlist: {}", playlist); debug!("Playlist: {}", playlist);
// Extract video playlist dir to dotenv // Extract video playlist dir to dotenv
if !playlist.starts_with("tmp") && is_valid_full_path(&app_state.base_path, playlist).is_some() if !playlist.starts_with("tmp") && is_valid_full_path(&app_state.base_path, playlist, false).is_some()
{ {
HttpResponse::BadRequest().finish() HttpResponse::BadRequest().finish()
} else if let Ok(file) = NamedFile::open(playlist) { } else if let Ok(file) = NamedFile::open(playlist) {