Skip to content

Commit

Permalink
Avoid using shell when spawning subprocess for git
Browse files Browse the repository at this point in the history
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"
  • Loading branch information
lieryan authored and David-Wobrock committed Feb 4, 2024
1 parent fff081f commit d677c08
Showing 1 changed file with 15 additions and 5 deletions.
20 changes: 15 additions & 5 deletions src/django_migration_linter/migration_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,21 @@ def _gather_migrations_git(
) -> 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
):
Expand Down

0 comments on commit d677c08

Please sign in to comment.