Skip to content

Commit e4edf56

Browse files
committed
PR fixes
1 parent 5fc85b7 commit e4edf56

1 file changed

Lines changed: 43 additions & 10 deletions

File tree

crates/bw/src/tools/send.rs

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -283,19 +283,20 @@ impl BwCommand for SendArgs {
283283
// `LoggedIn`. The integration tests in `tests/send.rs` assert that supplying
284284
// `encoded_json` to `create`/`edit` returns the "not yet implemented" error
285285
// even when the caller is logged out — that signals the input is unsupported
286-
// rather than burying it behind a confusing auth error.
286+
// rather than burying it behind a confusing auth error. Tracked under PM-39240;
287+
// kept out of the message itself since it ships in the production CLI output.
287288
Some(SendCommands::Create(args)) if args.encoded_json.is_some() => Err(eyre!(
288-
"`encoded_json` input on `bw send create` is not yet implemented (tracked under PM-39240)."
289+
"`encoded_json` input on `bw send create` is not yet implemented."
289290
)),
290291
Some(SendCommands::Edit(args)) if args.encoded_json.is_some() => Err(eyre!(
291-
"`encoded_json` input on `bw send edit` is not yet implemented (tracked under PM-39240)."
292+
"`encoded_json` input on `bw send edit` is not yet implemented."
292293
)),
293294
// `--output` on `get` similarly fails before the auth check: silently
294295
// emitting JSON to stdout while the requested file path goes uncreated would
295-
// be a worse UX than an explicit "not implemented" error.
296-
Some(SendCommands::Get(args)) if args.output_path.is_some() => Err(eyre!(
297-
"`--output` on `bw send get` is not yet implemented (tracked under PM-34718)."
298-
)),
296+
// be a worse UX than an explicit "not implemented" error. Tracked under PM-34718.
297+
Some(SendCommands::Get(args)) if args.output_path.is_some() => {
298+
Err(eyre!("`--output` on `bw send get` is not yet implemented."))
299+
}
299300
Some(SendCommands::List(args)) => args.dispatch(ctx).await,
300301
Some(SendCommands::Template(args)) => args.dispatch(ctx).await,
301302
Some(SendCommands::Get(args)) => args.dispatch(ctx).await,
@@ -346,9 +347,7 @@ impl BwCommand for SendReceiveArgs {
346347
type Client = AnyState;
347348

348349
async fn run(self, _: AnyState) -> CommandResult {
349-
Err(eyre!(
350-
"`bw send receive` is not yet implemented (tracked under PM-34718)."
351-
))
350+
Err(eyre!("`bw send receive` is not yet implemented."))
352351
}
353352
}
354353

@@ -732,6 +731,22 @@ async fn run_create(
732731
Ok(url.into())
733732
}
734733

734+
/// Reject `--file` paths containing a `..` segment before they reach the filesystem: a script
735+
/// that forwards an unsanitized path through to this CLI should not be able to resolve outside
736+
/// the directory it intended, even though `bw` itself only ever reads with the invoking user's
737+
/// own permissions.
738+
fn reject_path_traversal(path: &str) -> color_eyre::eyre::Result<()> {
739+
let has_parent_dir_segment = std::path::Path::new(path)
740+
.components()
741+
.any(|c| c == std::path::Component::ParentDir);
742+
if has_parent_dir_segment {
743+
return Err(eyre!(
744+
"Invalid --file path: {path} (path traversal segments are not allowed)."
745+
));
746+
}
747+
Ok(())
748+
}
749+
735750
/// Full file-send create pipeline:
736751
/// 1. Read the plaintext file bytes.
737752
/// 2. `create_file_send` encrypts them under the send key it derives, sends the ciphertext length
@@ -744,6 +759,8 @@ async fn run_create_file(
744759
request: SendAddRequest,
745760
path: &str,
746761
) -> color_eyre::eyre::Result<bitwarden_send::SendView> {
762+
reject_path_traversal(path)?;
763+
747764
// Read the plaintext before creating the send so a read failure aborts before we register a
748765
// send that would then have no content.
749766
let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;
@@ -1191,6 +1208,22 @@ mod tests {
11911208
assert!(compute_deletion_date(0).is_err());
11921209
}
11931210

1211+
// ---- reject_path_traversal ----
1212+
1213+
#[test]
1214+
fn reject_path_traversal_accepts_plain_paths() {
1215+
assert!(reject_path_traversal("secrets.txt").is_ok());
1216+
assert!(reject_path_traversal("/tmp/secrets.txt").is_ok());
1217+
assert!(reject_path_traversal("./dir/secrets.txt").is_ok());
1218+
}
1219+
1220+
#[test]
1221+
fn reject_path_traversal_rejects_parent_dir_segments() {
1222+
assert!(reject_path_traversal("../secrets.txt").is_err());
1223+
assert!(reject_path_traversal("dir/../../secrets.txt").is_err());
1224+
assert!(reject_path_traversal("/tmp/../etc/passwd").is_err());
1225+
}
1226+
11941227
// ---- build_auth ----
11951228

11961229
#[test]

0 commit comments

Comments
 (0)