Check source_file_path to ensure it is valid (#421)

* Add new variant `BinstallError::DuplicateSourceFilePath`
* Check `bin.source` in `collect_bin_files`
* Add new variant `BinstallError::InvalidSourceFilePath`
* Make sure generated source_file_path cannot access outside curdir
* Add new variant `BinstallError::EmptySourceFilePath`
* Ensure source_file_path is not empty

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
This commit is contained in:
Jiahao XU 2022-09-25 23:32:34 +10:00 committed by GitHub
parent e034d69e12
commit 3da5cb9d9c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 72 additions and 2 deletions

View file

@ -1,11 +1,12 @@
use std::{
borrow::Cow,
path::{Path, PathBuf},
path::{Component, Path, PathBuf},
};
use cargo_toml::Product;
use compact_str::CompactString;
use log::debug;
use normalize_path::NormalizePath;
use serde::Serialize;
use tinytemplate::TinyTemplate;
@ -15,6 +16,18 @@ use crate::{
manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
};
/// Return true if the path does not look outside of current dir
///
/// * `path` - must be normalized before passing to this function
fn is_valid_path(path: &Path) -> bool {
!matches!(
path.components().next(),
// normalized path cannot have curdir or parentdir,
// so checking prefix/rootdir is enough.
Some(Component::Prefix(..) | Component::RootDir)
)
}
pub struct BinFile {
pub base_name: CompactString,
pub source: PathBuf,
@ -47,7 +60,23 @@ impl BinFile {
// Generate install paths
// Source path is the download dir + the generated binary path
let source_file_path = if let Some(bin_dir) = &data.meta.bin_dir {
ctx.render(bin_dir)?
let path = ctx.render(bin_dir)?;
let path_normalized = Path::new(&path).normalize();
if path_normalized.components().next().is_none() {
return Err(BinstallError::EmptySourceFilePath);
}
if !is_valid_path(&path_normalized) {
return Err(BinstallError::InvalidSourceFilePath {
path: path_normalized.into_owned(),
});
}
match path_normalized {
Cow::Borrowed(..) => path,
Cow::Owned(path) => path.to_string_lossy().into_owned(),
}
} else {
let name = ctx.name;
let target = ctx.target;

View file

@ -284,6 +284,33 @@ pub enum BinstallError {
#[diagnostic(severity(error), code(binstall::cargo_manifest))]
CargoTomlMissingPackage(CompactString),
/// bin-dir configuration provided generates duplicate source path.
///
/// - Code: `binstall::cargo_manifest`
/// - Exit: 90
#[error("bin-dir configuration provided generates duplicate source path: {path}")]
#[diagnostic(severity(error), code(binstall::SourceFilePath))]
DuplicateSourceFilePath { path: PathBuf },
/// bin-dir configuration provided generates source path outside
/// of the temporary dir.
///
/// - Code: `binstall::cargo_manifest`
/// - Exit: 91
#[error(
"bin-dir configuration provided generates source path outside of the temporary dir: {path}"
)]
#[diagnostic(severity(error), code(binstall::SourceFilePath))]
InvalidSourceFilePath { path: PathBuf },
/// bin-dir configuration provided generates empty source path.
///
/// - Code: `binstall::cargo_manifest`
/// - Exit: 92
#[error("bin-dir configuration provided generates empty source path")]
#[diagnostic(severity(error), code(binstall::SourceFilePath))]
EmptySourceFilePath,
/// A wrapped error providing the context of which crate the error is about.
#[error("for crate {crate_name}")]
CrateContext {
@ -319,6 +346,9 @@ impl BinstallError {
NoViableTargets => 87,
BinFileNotFound(_) => 88,
CargoTomlMissingPackage(_) => 89,
DuplicateSourceFilePath { .. } => 90,
InvalidSourceFilePath { .. } => 91,
EmptySourceFilePath => 92,
CrateContext { error, .. } => error.exit_number(),
};

View file

@ -1,4 +1,5 @@
use std::{
collections::BTreeSet,
path::{Path, PathBuf},
sync::Arc,
};
@ -356,6 +357,16 @@ fn collect_bin_files(
.map(|p| bins::BinFile::from_product(&bin_data, p))
.collect::<Result<Vec<_>, BinstallError>>()?;
let mut source_set = BTreeSet::new();
for bin in &bin_files {
if !source_set.insert(&bin.source) {
return Err(BinstallError::DuplicateSourceFilePath {
path: bin.source.clone(),
});
}
}
Ok(bin_files)
}