From 82019c8d806036e60046bcfda5135694e27c4212 Mon Sep 17 00:00:00 2001 From: Adam Poulemanos Date: Mon, 29 Jun 2026 22:48:47 -0400 Subject: [PATCH] test(p2): assert command/flag-injection containment for name/url/.gitmodules (#62 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit security_tests.rs had only 2 tests (both the same `-`-prefixed-path vector). The #62 audit (P2) calls out missing command/flag-injection and malicious-.gitmodules coverage. Adds two characterization tests proving submod's no-shell design holds end-to-end: - test_flag_like_name_and_url_do_not_inject_commands: a CVE-2018-17456-class `--upload-pack=` payload as a submodule NAME, and an `ext::sh -c` payload as the URL, must never execute. Sentinel files (relative in the worktree, absolute) are asserted absent. Non-vacuous: an unsafe shell-out would create them. - test_generate_config_from_malicious_gitmodules_does_not_execute: a hostile .gitmodules fed to `generate-config --from-setup` is parsed as data only — no sentinel is created, and the generated config provably contains the hostile values as inert strings (so the parse path actually ran). Behavior probed against the real binary first (both vectors are already safe; these lock that in). Full suite 557 pass; fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01T8D5ZK1473YCiZkbueAY2X --- tests/security_tests.rs | 101 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/tests/security_tests.rs b/tests/security_tests.rs index e601887..52439ba 100644 --- a/tests/security_tests.rs +++ b/tests/security_tests.rs @@ -4,6 +4,8 @@ //! Security tests to ensure robustness against various attack vectors. +use std::fs; + mod common; use common::TestHarness; @@ -98,4 +100,103 @@ mod tests { // Verify it worked assert!(harness.dir_exists("sub/-sparse/src")); } + + /// CVE-2018-17456-class option injection: a submodule name or URL that mimics + /// a `git clone`/`git submodule` flag (e.g. `--upload-pack=`) must be + /// treated as inert data, never as an option that triggers command execution. + /// + /// submod drives git via gix/git2 (no shell) and uses `--` before the URL/path + /// on the CLI last-resort path, so the payload should never run. This test is + /// non-vacuous: if any code path ever shelled the name/URL out unsafely, the + /// sentinel file would be created. + #[test] + fn test_flag_like_name_and_url_do_not_inject_commands() { + let harness = TestHarness::new().expect("Failed to create test harness"); + harness.init_git_repo().expect("Failed to init git repo"); + + let remote_repo = harness + .create_test_remote("inject-remote") + .expect("Failed to create remote"); + let remote_url = format!("file://{}", remote_repo.display()); + + // Sentinels: `touch ` would land in cwd (the working tree); + // `touch ` would land at a fixed path. Neither must appear. + let rel_sentinel = harness.work_dir.join("INJECTED_BY_NAME"); + let abs_sentinel = harness.work_dir.join("INJECTED_BY_URL"); + assert!(!rel_sentinel.exists() && !abs_sentinel.exists()); + + // Flag-like NAME (passed via `--name=` so clap accepts the leading dashes). + let _ = harness + .run_submod(&[ + "add", + &remote_url, + "--name=--upload-pack=touch INJECTED_BY_NAME", + "--path", + "lib/inj-name", + ]) + .expect("Failed to run submod"); + assert!( + !rel_sentinel.exists(), + "a flag-like submodule name must not inject a command" + ); + + // Malicious transport URL (ext:: would run a shell command if honored). + let evil_url = format!("ext::sh -c \"touch {}\"", abs_sentinel.display()); + let _ = harness + .run_submod(&[ + "add", + &evil_url, + "--name", + "inj-url", + "--path", + "lib/inj-url", + ]) + .expect("Failed to run submod"); + assert!( + !abs_sentinel.exists(), + "a malicious transport URL must not execute a command" + ); + } + + /// A hostile `.gitmodules` fed to `generate-config --from-setup` must be parsed + /// as data only: its url/branch values are serialized verbatim into the output + /// config, never executed. Non-vacuous: the generated file must actually contain + /// the hostile values (proving the parse path ran) while no sentinel is created. + #[test] + fn test_generate_config_from_malicious_gitmodules_does_not_execute() { + let harness = TestHarness::new().expect("Failed to create test harness"); + harness.init_git_repo().expect("Failed to init git repo"); + + let sentinel = harness.work_dir.join("GC_INJECTED"); + let gitmodules = format!( + "[submodule \"evil\"]\n\tpath = lib/evil\n\turl = ext::sh -c \"touch {}\"\n\tbranch = --upload-pack=touch {}\n", + sentinel.display(), + sentinel.display() + ); + fs::write(harness.work_dir.join(".gitmodules"), gitmodules) + .expect("Failed to write .gitmodules"); + + harness + .run_submod(&[ + "generate-config", + "--from-setup", + "--output", + "out.toml", + "--force", + ]) + .expect("Failed to run submod"); + + assert!( + !sentinel.exists(), + "generate-config must not execute values read from .gitmodules" + ); + + // Non-vacuity: the hostile entry was actually parsed and captured as data. + let generated = fs::read_to_string(harness.work_dir.join("out.toml")) + .expect("generate-config should have written the output config"); + assert!( + generated.contains("ext::sh -c"), + "expected the hostile url to be captured as inert data, got:\n{generated}" + ); + } }