Skip to content

Commit a536ca4

Browse files
authored
Prevent temp file swapping in tx edit command. (#2521)
1 parent 12f76e4 commit a536ca4

1 file changed

Lines changed: 59 additions & 7 deletions

File tree

  • cmd/soroban-cli/src/commands/tx

cmd/soroban-cli/src/commands/tx/edit.rs

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ use std::{
66
process::{self},
77
};
88

9+
use tempfile::TempDir;
10+
911
use serde_json::json;
1012
use stellar_xdr::curr;
1113

@@ -52,14 +54,14 @@ impl Cmd {
5254
xdr_to_json::<curr::TransactionEnvelope>(input)?
5355
};
5456

55-
let path = tmp_file(&json)?;
57+
let (_temp_dir, path) = tmp_file(&json)?;
5658
let editor = get_editor();
5759

60+
print.infoln(format!("Editing transaction at {}", path.display()));
5861
open_editor(&print, &editor, &path)?;
5962

6063
let contents = fs::read_to_string(&path)?;
6164
let xdr = json_to_xdr::<curr::TransactionEnvelope>(&contents)?;
62-
fs::remove_file(&path)?;
6365

6466
println!("{xdr}");
6567

@@ -73,15 +75,30 @@ struct Editor {
7375
args: Vec<String>,
7476
}
7577

76-
fn tmp_file(contents: &str) -> Result<PathBuf, Error> {
77-
let temp_dir = env::current_dir().unwrap_or(env::temp_dir());
78-
let file_name = format!("stellar-xdr-{}.json", rand::random::<u64>());
79-
let path = temp_dir.join(file_name);
78+
fn tmp_file(contents: &str) -> Result<(TempDir, PathBuf), Error> {
79+
let temp_dir = tempfile::Builder::new()
80+
.prefix("stellar-tx-edit-")
81+
.tempdir()?;
82+
let path = temp_dir.path().join("edit.json");
8083

84+
#[cfg(unix)]
85+
let mut file = {
86+
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
87+
fs::set_permissions(temp_dir.path(), fs::Permissions::from_mode(0o700))?;
88+
fs::OpenOptions::new()
89+
.write(true)
90+
.create(true)
91+
.truncate(true)
92+
.mode(0o600)
93+
.open(&path)?
94+
};
95+
96+
#[cfg(not(unix))]
8197
let mut file = fs::File::create(&path)?;
98+
8299
file.write_all(contents.as_bytes())?;
83100

84-
Ok(path)
101+
Ok((temp_dir, path))
85102
}
86103

87104
fn get_editor() -> Editor {
@@ -193,3 +210,38 @@ fn default_json() -> String {
193210
"#
194211
)
195212
}
213+
214+
#[cfg(test)]
215+
mod tests {
216+
use super::*;
217+
218+
#[test]
219+
fn tmp_file_uses_private_tempdir() {
220+
let contents = r#"{"test": true}"#;
221+
let (temp_dir, path) = tmp_file(contents).expect("tmp_file failed");
222+
223+
// File must exist inside the tempdir, not in CWD
224+
assert!(path.starts_with(temp_dir.path()));
225+
assert_ne!(temp_dir.path(), env::current_dir().unwrap());
226+
227+
// Contents must match
228+
let read_back = fs::read_to_string(&path).expect("read failed");
229+
assert_eq!(read_back, contents);
230+
}
231+
232+
#[cfg(unix)]
233+
#[test]
234+
fn tmp_file_has_restricted_permissions() {
235+
use std::os::unix::fs::PermissionsExt;
236+
237+
let (temp_dir, path) = tmp_file("{}").expect("tmp_file failed");
238+
239+
let file_meta = fs::metadata(&path).expect("file metadata failed");
240+
let file_mode = file_meta.permissions().mode() & 0o777;
241+
assert_eq!(file_mode, 0o600, "file permissions should be 0o600");
242+
243+
let dir_meta = fs::metadata(temp_dir.path()).expect("dir metadata failed");
244+
let dir_mode = dir_meta.permissions().mode() & 0o777;
245+
assert_eq!(dir_mode, 0o700, "tempdir permissions should be 0o700");
246+
}
247+
}

0 commit comments

Comments
 (0)