Fix symlink attack via predictable /tmp path in write_raw_config_to_disk - #358
Merged
Conversation
write_raw_config_to_disk wrote the serialized raw config to a path derived solely from run_id, which is attacker/user-controllable via --run-id or SCOPE_RUN_ID. Combined with plain File::create following symlinks, this let anyone who could predict the path pre-plant a symlink and have scope clobber an arbitrary file on write. The file is purely transient IPC to a child process spawned right after, so swap it for a securely-generated unique tempfile instead of a run_id-derived name. Move tempfile from dev-dependencies to dependencies since it's now needed at runtime. DryRun Security finding from PR #356 review.
|
This pull request introduces a low-severity security issue where the application creates a temporary directory with default permissions, potentially allowing a local attacker to manipulate the configuration file. The finding is non-blocking and does not exceed the risk threshold.
Insecure Temp Directory in
|
| Vulnerability | Insecure Temp Directory |
|---|---|
| Description | The application creates a directory at '/tmp/scope' using fs::create_dir_all, which relies on the system umask and does not explicitly set secure permissions (e.g., 0o700). While the application uses tempfile to create the configuration file within this directory, the directory itself is created with default permissions. A local attacker could pre-create /tmp/scope with world-writable permissions, potentially allowing them to delete or replace the configuration file before the application consumes it, if the application does not verify the directory's ownership or permissions. |
scope/src/shared/config_load.rs
Lines 180 to 183 in 4f326e4
Comment to provide feedback on these findings.
Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]
Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing
All finding details can be found in the DryRun Security Dashboard.
rubberduck203
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
write_raw_config_to_diskwrote the serialized raw config to/tmp/scope/config-{run_id}.jsonvia plainFile::create, which follows symlinksrun_idis attacker/user-controllable via--run-idorSCOPE_RUN_ID, making the exact path predictable ahead of time; on a shared multi-user box or CI runner, an attacker could pre-plant a symlink there to have scope overwrite an arbitrary filetempfile::Builderto create a securely-generated, unpredictable file under/tmp/scope, persisted viaNamedTempFile::keep()so it survives past this function (it's read by a child process spawned right after inexec_sub_command)tempfilefrom[dev-dependencies]to[dependencies]inCargo.tomlsince it's now needed at runtimeDryRun Security finding surfaced during review of #356 (that PR's own diff is untouched here; this fixes a pre-existing issue in the surrounding file that the security scanner flagged while scanning the whole changed file).
Test plan
cargo buildcargo test(full suite, 195+ tests pass)cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningsscope externalagainst thetests/test-cases/command-pathsfixture with an instrumented script; confirmed$SCOPE_CONFIG_JSONpoints to a randomly-named file (notrun_id-derived), created with0600permissions, containing valid config JSON readable by the child process