Skip to content

fix(security): resolve TOCTOU race condition in atomic writes #402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alysajad wants to merge 2 commits into
base: main
Choose a base branch
from
+30 −21
Open

fix(security): resolve TOCTOU race condition in atomic writes #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e40047
fix(security): resolve TOCTOU race condition in atomic writes
alysajad Mar 11, 2026
1fc72ee
docs: add changeset for TOCTOU security fix
alysajad Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/resolute-toctou.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"gws": patch
---

Resolve TOCTOU race condition in `fs_util::atomic_write` and `atomic_write_async` to securely enforce 0600 file permissions upon file creation, preventing intermediate local read access to secrets.
12 changes: 0 additions & 12 deletions src/credential_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,18 +391,6 @@ pub fn save_encrypted(json: &str) -> anyhow::Result<PathBuf> {
crate::fs_util::atomic_write(&path, &encrypted)
.map_err(|e| anyhow::anyhow!("Failed to write credentials: {e}"))?;

// Set permissions to 600 on Unix (contains secrets)
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if let Err(e) = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)) {
eprintln!(
"Warning: failed to set file permissions on {}: {e}",
path.display()
);
}
}

Ok(path)
}

Expand Down
27 changes: 25 additions & 2 deletions src/fs_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,19 @@ pub fn atomic_write(path: &Path, data: &[u8]) -> io::Result<()> {
.map(|p| p.join(&tmp_name))
.unwrap_or_else(|| std::path::PathBuf::from(&tmp_name));

std::fs::write(&tmp_path, data)?;
{
use std::io::Write;
let mut opts = std::fs::OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.mode(0o600);
}
let mut file = opts.open(&tmp_path)?;
Comment on lines +45 to +52
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The use of a predictable temporary filename (path.tmp) combined with create(true).truncate(true) without O_EXCL (via create_new(true)) introduces a local race condition.

On Unix-like systems, the mode(0o600) option in OpenOptions only applies if the file is newly created by the open call. If a local attacker pre-creates the temporary file (e.g., credentials.enc.tmp) with permissive permissions (like 0666) before the application runs, the open call will succeed, truncate the existing file, and write sensitive data to it without changing its permissions. This allows the attacker to read the secrets written to the file.

Furthermore, because open follows symbolic links by default, an attacker could create a symlink at the predictable temporary path pointing to a sensitive file owned by the user, leading to an arbitrary file overwrite.

While this PR attempts to fix a TOCTOU by using OpenOptions::mode, it is incomplete because it doesn't account for the case where the file already exists. The removal of the post-write set_permissions fallback in credential_store.rs and oauth_config.rs makes this vulnerability more critical as there is no longer a secondary check to restrict permissions.

Suggested change
let mut opts = std::fs::OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.mode(0o600);
}
let mut file = opts.open(&tmp_path)?;
let mut opts = std::fs::OpenOptions::new();
opts.write(true).create_new(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.mode(0o600);
}
let mut file = opts.open(&tmp_path)?;

file.write_all(data)?;
file.sync_all()?;
}
std::fs::rename(&tmp_path, path)?;
Ok(())
}
Expand All @@ -56,7 +68,18 @@ pub async fn atomic_write_async(path: &Path, data: &[u8]) -> io::Result<()> {
.map(|p| p.join(&tmp_name))
.unwrap_or_else(|| std::path::PathBuf::from(&tmp_name));

tokio::fs::write(&tmp_path, data).await?;
{
use tokio::io::AsyncWriteExt;
let mut opts = tokio::fs::OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
opts.mode(0o600);
}
let mut file = opts.open(&tmp_path).await?;
file.write_all(data).await?;
file.sync_all().await?;
}
tokio::fs::rename(&tmp_path, path).await?;
Ok(())
}
Expand Down
7 changes: 0 additions & 7 deletions src/oauth_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,6 @@ pub fn save_client_config(
crate::fs_util::atomic_write(&path, json.as_bytes())
.map_err(|e| anyhow::anyhow!("Failed to write client config: {e}"))?;

// Set file permissions to 600 on Unix (contains secrets)
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))?;
}

Ok(path)
}

Expand Down
Loading