From 6cbba39f532371189aaf6b920112b2a97d71390e Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Fri, 11 Mar 2022 17:00:50 -0600 Subject: [PATCH 1/4] test(test_hg): Alias command injection recreation in URLs --- tests/test_hg.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/test_hg.py b/tests/test_hg.py index a73f7372..bf292284 100644 --- a/tests/test_hg.py +++ b/tests/test_hg.py @@ -5,7 +5,7 @@ import pytest -from libvcs.shortcuts import create_repo_from_pip_url +from libvcs.shortcuts import create_repo, create_repo_from_pip_url from libvcs.util import run, which if not which("hg"): @@ -72,3 +72,26 @@ def test_repo_mercurial(tmp_path: pathlib.Path, repos_path, hg_remote): ) assert mercurial_repo.get_revision() == test_repo_revision + + +def test_vulnerability_2022_03_12_command_injection( + monkeypatch: pytest.MonkeyPatch, + user_path: pathlib.Path, + tmp_path: pathlib.Path, + hg_remote, +): + """Prevent hg aliases from executed arbitrary commands via URLs. + + As of 0.11 this code path is/was only executed via .obtain(), so this only would + effect explicit invocation of .object() or update_repo() of uncloned destination. + """ + random_dir = tmp_path / "random" + random_dir.mkdir() + monkeypatch.chdir(str(random_dir)) + mercurial_repo = create_repo( + url="--config=alias.clone=!touch ./HELLO", vcs="hg", repo_dir="./" + ) + with pytest.raises(Exception): + mercurial_repo.update_repo() + + assert pathlib.Path(random_dir / "HELLO").exists() From c8a2ca6426f6d388a6604b451dac02320da51853 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 12 Mar 2022 09:55:40 -0600 Subject: [PATCH 2/4] tests(hg): Check for URL command injection vuln --- tests/test_hg.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_hg.py b/tests/test_hg.py index bf292284..7d267f90 100644 --- a/tests/test_hg.py +++ b/tests/test_hg.py @@ -94,4 +94,6 @@ def test_vulnerability_2022_03_12_command_injection( with pytest.raises(Exception): mercurial_repo.update_repo() - assert pathlib.Path(random_dir / "HELLO").exists() + assert not pathlib.Path( + random_dir / "HELLO" + ).exists(), "Prevent command injection in hg aliases" From 3f4e93ed5c22ec5c8b54a7a24b225dc24b38283a Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 12 Mar 2022 09:57:04 -0600 Subject: [PATCH 3/4] fix(hg): Command injection vulnerability in URLs via alias > create_repo( > url="--config=alias.clone=!touch ./HELLO", vcs="hg", repo_dir="./" > ) Credit: Alessio Della Libera via Snyk --- libvcs/hg.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libvcs/hg.py b/libvcs/hg.py index a89ea82a..5ec53e03 100644 --- a/libvcs/hg.py +++ b/libvcs/hg.py @@ -26,7 +26,9 @@ def __init__(self, url, repo_dir, **kwargs): def obtain(self): self.ensure_dir() - self.run(["clone", "--noupdate", "-q", self.url, self.path]) + # Double hyphens between [OPTION]... -- SOURCE [DEST] prevent command injections + # via aliases + self.run(["clone", "--noupdate", "-q", "--", self.url, self.path]) self.run(["update", "-q"]) def get_revision(self): From 66640aedbcaec14bda1b73077f1fefc3dd4157ee Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 12 Mar 2022 10:03:28 -0600 Subject: [PATCH 4/4] docs(CHANGES): Note vulnerability fix --- CHANGES | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES b/CHANGES index da3752c2..e546ef7d 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,12 @@ - _Add your latest changes from PRs here_ +### Potential command injection via mercurial URLs + +- By setting a mercurial URL with an alias it is possible to execute arbitrary shell commands via + `.obtain()` or in the case of uncloned destinations, `.update_repo()`. (#306, credit: Alessio + Della Libera) + ### Development - Run pyupgrade formatting (#305)