fix: prevent relative directory traversal from crashing (#12438)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

- fixes #11922
- fixes #12203

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
This is a rewrite for some parts of the recursive completion system. The
Rust `std::path` structures often ignores things like a trailing `.`
because for a complete path, it implies the current directory. We are
replacing the use of some of these structs for Strings.

A side effect is the slashes being normalized in Windows. For example if
we were to type `foo/bar/b`, it would complete it to `foo\bar\baz`
because a backward slash is the main separator in windows.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Relative paths are preserved. `..`s in the paths won't eagerly show
completions from the parent path. For example, `asd/foo/../b` will now
complete to `asd/foo/../bar` instead of `asd/bar`.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This commit is contained in:
Himadri Bhattacharjee 2024-05-04 06:47:50 +05:30 committed by GitHub
parent a1287f7b3f
commit 35a0f7a369
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 117 additions and 95 deletions

View file

@ -7,40 +7,65 @@ use nu_protocol::{
Span,
};
use nu_utils::get_ls_colors;
use std::{
ffi::OsStr,
path::{is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP},
use std::path::{
is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP, MAIN_SEPARATOR_STR,
};
#[derive(Clone, Default)]
pub struct PathBuiltFromString {
parts: Vec<String>,
isdir: bool,
}
fn complete_rec(
partial: &[String],
partial: &[&str],
built: &PathBuiltFromString,
cwd: &Path,
options: &CompletionOptions,
dir: bool,
isdir: bool,
) -> Vec<PathBuf> {
) -> Vec<PathBuiltFromString> {
let mut completions = vec![];
if let Ok(result) = cwd.read_dir() {
for entry in result.filter_map(|e| e.ok()) {
let entry_name = entry.file_name().to_string_lossy().into_owned();
let path = entry.path();
if let Some((&base, rest)) = partial.split_first() {
if (base == "." || base == "..") && (isdir || !rest.is_empty()) {
let mut built = built.clone();
built.parts.push(base.to_string());
built.isdir = true;
return complete_rec(rest, &built, cwd, options, dir, isdir);
}
}
if !dir || path.is_dir() {
match partial.first() {
Some(base) if matches(base, &entry_name, options) => {
let partial = &partial[1..];
if !partial.is_empty() || isdir {
completions.extend(complete_rec(partial, &path, options, dir, isdir));
if entry_name.eq(base) {
break;
}
let mut built_path = cwd.to_path_buf();
for part in &built.parts {
built_path.push(part);
}
let Ok(result) = built_path.read_dir() else {
return completions;
};
for entry in result.filter_map(|e| e.ok()) {
let entry_name = entry.file_name().to_string_lossy().into_owned();
let entry_isdir = entry.path().is_dir();
let mut built = built.clone();
built.parts.push(entry_name.clone());
built.isdir = entry_isdir;
if !dir || entry_isdir {
match partial.split_first() {
Some((base, rest)) => {
if matches(base, &entry_name, options) {
if !rest.is_empty() || isdir {
completions
.extend(complete_rec(rest, &built, cwd, options, dir, isdir));
} else {
completions.push(path)
completions.push(built);
}
}
None => completions.push(path),
_ => {}
}
None => {
completions.push(built);
}
}
}
@ -48,33 +73,23 @@ fn complete_rec(
completions
}
#[derive(Debug)]
enum OriginalCwd {
None,
Home(PathBuf),
Some(PathBuf),
// referencing a single local file
Local(PathBuf),
Home,
Prefix(String),
}
impl OriginalCwd {
fn apply(&self, p: &Path) -> String {
let mut ret = match self {
Self::None => p.to_string_lossy().into_owned(),
Self::Some(base) => pathdiff::diff_paths(p, base)
.unwrap_or(p.to_path_buf())
.to_string_lossy()
.into_owned(),
Self::Home(home) => match p.strip_prefix(home) {
Ok(suffix) => format!("~{}{}", SEP, suffix.to_string_lossy()),
_ => p.to_string_lossy().into_owned(),
},
Self::Local(base) => Path::new(".")
.join(pathdiff::diff_paths(p, base).unwrap_or(p.to_path_buf()))
.to_string_lossy()
.into_owned(),
fn apply(&self, mut p: PathBuiltFromString) -> String {
match self {
Self::None => {}
Self::Home => p.parts.insert(0, "~".to_string()),
Self::Prefix(s) => p.parts.insert(0, s.clone()),
};
if p.is_dir() {
let mut ret = p.parts.join(MAIN_SEPARATOR_STR);
if p.isdir {
ret.push(SEP);
}
ret
@ -116,79 +131,67 @@ pub fn complete_item(
};
get_ls_colors(ls_colors_env_str)
});
let mut cwd = cwd_pathbuf.clone();
let mut prefix_len = 0;
let mut original_cwd = OriginalCwd::None;
let mut components_vec: Vec<Component> = Path::new(&partial).components().collect();
// Path components that end with a single "." get normalized away,
// so if the partial path ends in a literal "." we must add it back in manually
if partial.ends_with('.') && partial.len() > 1 {
components_vec.push(Component::Normal(OsStr::new(".")));
};
let mut components = components_vec.into_iter().peekable();
let mut cwd = match components.peek().cloned() {
let mut components = Path::new(&partial).components().peekable();
match components.peek().cloned() {
Some(c @ Component::Prefix(..)) => {
// windows only by definition
components.next();
if let Some(Component::RootDir) = components.peek().cloned() {
components.next();
};
[c, Component::RootDir].iter().collect()
cwd = [c, Component::RootDir].iter().collect();
prefix_len = c.as_os_str().len();
original_cwd = OriginalCwd::Prefix(c.as_os_str().to_string_lossy().into_owned());
}
Some(c @ Component::RootDir) => {
components.next();
PathBuf::from(c.as_os_str())
// This is kind of a hack. When joining an empty string with the rest,
// we add the slash automagically
cwd = PathBuf::from(c.as_os_str());
prefix_len = 1;
original_cwd = OriginalCwd::Prefix(String::new());
}
Some(Component::Normal(home)) if home.to_string_lossy() == "~" => {
components.next();
original_cwd = OriginalCwd::Home(home_dir().unwrap_or(cwd_pathbuf.clone()));
home_dir().unwrap_or(cwd_pathbuf)
}
Some(Component::CurDir) => {
components.next();
original_cwd = match components.peek().cloned() {
Some(Component::Normal(_)) | None => OriginalCwd::Local(cwd_pathbuf.clone()),
_ => OriginalCwd::Some(cwd_pathbuf.clone()),
};
cwd_pathbuf
}
_ => {
original_cwd = OriginalCwd::Some(cwd_pathbuf.clone());
cwd_pathbuf
cwd = home_dir().unwrap_or(cwd_pathbuf);
prefix_len = 1;
original_cwd = OriginalCwd::Home;
}
_ => {}
};
let mut partial = vec![];
let after_prefix = &partial[prefix_len..];
let partial: Vec<_> = after_prefix
.strip_prefix(is_separator)
.unwrap_or(after_prefix)
.split(is_separator)
.filter(|s| !s.is_empty())
.collect();
for component in components {
match component {
Component::Prefix(..) => unreachable!(),
Component::RootDir => unreachable!(),
Component::CurDir => {}
Component::ParentDir => {
if partial.pop().is_none() {
cwd.pop();
}
}
Component::Normal(c) => partial.push(c.to_string_lossy().into_owned()),
}
}
complete_rec(partial.as_slice(), &cwd, options, want_directory, isdir)
.into_iter()
.map(|p| {
let path = original_cwd.apply(&p);
let style = ls_colors.as_ref().map(|lsc| {
lsc.style_for_path_with_metadata(
&path,
std::fs::symlink_metadata(&path).ok().as_ref(),
)
complete_rec(
partial.as_slice(),
&PathBuiltFromString::default(),
&cwd,
options,
want_directory,
isdir,
)
.into_iter()
.map(|p| {
let path = original_cwd.apply(p);
let style = ls_colors.as_ref().map(|lsc| {
lsc.style_for_path_with_metadata(&path, std::fs::symlink_metadata(&path).ok().as_ref())
.map(lscolors::Style::to_nu_ansi_term_style)
.unwrap_or_default()
});
(span, escape_path(path, want_directory), style)
})
.collect()
});
(span, escape_path(path, want_directory), style)
})
.collect()
}
// Fix files or folders with quotes or hashes

View file

@ -334,7 +334,26 @@ fn partial_completions() {
let suggestions = completer.complete(&target_dir, target_dir.len());
// Create the expected values
let expected_paths: Vec<String> = vec![file(dir.join("final_partial").join("somefile"))];
let expected_paths: Vec<String> = vec![
file(
dir.join("partial_a")
.join("..")
.join("final_partial")
.join("somefile"),
),
file(
dir.join("partial_b")
.join("..")
.join("final_partial")
.join("somefile"),
),
file(
dir.join("partial_c")
.join("..")
.join("final_partial")
.join("somefile"),
),
];
// Match the results
match_suggestions(expected_paths, suggestions);