Skip to content

Commit

Permalink
Check source_file_path to ensure it is valid (rust-lang#421)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
NobodyXu authored Sep 25, 2022
1 parent e034d69 commit 3da5cb9
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
33 changes: 31 additions & 2 deletions crates/binstalk/src/bins.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions crates/binstalk/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -319,6 +346,9 @@ impl BinstallError {
NoViableTargets => 87,
BinFileNotFound(_) => 88,
CargoTomlMissingPackage(_) => 89,
DuplicateSourceFilePath { .. } => 90,
InvalidSourceFilePath { .. } => 91,
EmptySourceFilePath => 92,
CrateContext { error, .. } => error.exit_number(),
};

Expand Down
11 changes: 11 additions & 0 deletions crates/binstalk/src/ops/resolve.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{
collections::BTreeSet,
path::{Path, PathBuf},
sync::Arc,
};
Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 3da5cb9

Please sign in to comment.