From d677c08a070f020123449ccfbc9bfb3b3774eb63 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 11 Jan 2024 18:58:33 +1100 Subject: [PATCH] Avoid using shell when spawning subprocess for git This avoids security shenanigans that can come if your `git-commit-id` may come from potentially untrusted source (e.g. an API), an attacker could trick you to run shell commands like: ./manage.py lintmigrations --git-commit-id '; rm -rf dangerous' or a malicious project may set a config like: [tool.django_migration_linter] git_commit_id = "; rm -rf dangerous" --- .../migration_linter.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/django_migration_linter/migration_linter.py b/src/django_migration_linter/migration_linter.py index 5ba9605..92e3057 100644 --- a/src/django_migration_linter/migration_linter.py +++ b/src/django_migration_linter/migration_linter.py @@ -374,11 +374,21 @@ class MigrationLinter: ) -> Iterable[Migration]: migrations = [] # Get changes since specified commit - git_diff_command = ( - "cd {} && git diff --relative --name-only --diff-filter=AR {}" - ).format(self.django_path, git_commit_id) - logger.info(f"Executing {git_diff_command}") - diff_process = Popen(git_diff_command, shell=True, stdout=PIPE, stderr=PIPE) + git_diff_command = [ + "git", + "diff", + "--relative", + "--name-only", + "--diff-filter=AR", + git_commit_id, + ] + logger.info(f"Executing {git_diff_command} (in {self.django_path})") + diff_process = Popen( + git_diff_command, + stdout=PIPE, + stderr=PIPE, + cwd=self.django_path, + ) for line in map( clean_bytes_to_str, diff_process.stdout.readlines() # type: ignore ):