diff --git a/git_stacktrace/cmd.py b/git_stacktrace/cmd.py index 34b33df..d7c028c 100644 --- a/git_stacktrace/cmd.py +++ b/git_stacktrace/cmd.py @@ -51,8 +51,11 @@ def main(): logging.getLogger().setLevel(logging.DEBUG) if args.server: - print("Starting httpd on port %s..." % args.port) - httpd = make_server("", args.port, server.application) + # Bind to loopback only; the HTTP API accepts unauthenticated query + # params that are passed to git, so it must not be reachable on LAN. + host = "127.0.0.1" + print("Starting httpd on %s port %s..." % (host, args.port)) + httpd = make_server(host, args.port, server.application) try: httpd.serve_forever() except KeyboardInterrupt: diff --git a/git_stacktrace/git.py b/git_stacktrace/git.py index a45003a..9c8433c 100644 --- a/git_stacktrace/git.py +++ b/git_stacktrace/git.py @@ -15,6 +15,14 @@ CommitInfo = collections.namedtuple("CommitInfo", ["summary", "subject", "body", "url", "author", "date"]) +def validate_git_revision_arg(value, what="git argument"): + """Reject values git would treat as options (argument injection via leading '-').""" + if value is None or value == "": + return + if value.startswith("-"): + raise ValueError("Invalid %s %r: values must not start with '-'" % (what, value)) + + class GitFile(object): """Track filename and if file was added/removed or modified.""" @@ -74,6 +82,7 @@ def files_touched(git_range): Generate a dictionary of files modified by the commits in range """ + validate_git_revision_arg(git_range, "git range") cmd = "git", "log", "--pretty=%H", "--raw", git_range data = run_command(*cmd) commits = collections.defaultdict(list) @@ -99,6 +108,7 @@ def pickaxe(snippet, git_range, filename=None): Return list of commits that modified that snippet """ + validate_git_revision_arg(git_range, "git range") cmd = "git", "log", "-b", "--pretty=%H", "-S", str(snippet), git_range if filename: cmd = cmd + ( @@ -193,6 +203,7 @@ def valid_range(git_range): Returns True or False """ + validate_git_revision_arg(git_range, "git range") cmd = "git", "log", "--oneline", git_range data = run_command(*cmd) lines = data.splitlines() @@ -200,6 +211,8 @@ def valid_range(git_range): def convert_since(since, branch=None): + validate_git_revision_arg(since, "since") + validate_git_revision_arg(branch, "branch") cmd = "git", "log", "--pretty=%H", "--since=%s" % since if branch: cmd = cmd + (branch,) @@ -211,7 +224,9 @@ def convert_since(since, branch=None): def files(git_range): + validate_git_revision_arg(git_range, "git range") commit = git_range.split(".")[-1] + validate_git_revision_arg(commit, "git commit") cmd = "git", "ls-tree", "-r", "--name-only", commit data = run_command(*cmd) files = data.splitlines() diff --git a/git_stacktrace/server.py b/git_stacktrace/server.py index 6542bf0..6a25ea7 100644 --- a/git_stacktrace/server.py +++ b/git_stacktrace/server.py @@ -70,13 +70,19 @@ def validate(self): if self.type == "by-date": if not self.since: return "Missing `since` value. Plese specify a date." - self.git_range = api.convert_since(self.since, branch=self.branch) + try: + self.git_range = api.convert_since(self.since, branch=self.branch) + except ValueError as e: + return str(e) if not api.valid_range(self.git_range): return "Found no commits in '%s'" % self.git_range elif self.type == "by-range": self.git_range = self.range - if not api.valid_range(self.git_range): - return "Found no commits in '%s'" % self.git_range + try: + if not api.valid_range(self.git_range): + return "Found no commits in '%s'" % self.git_range + except ValueError as e: + return str(e) else: return "Invalid `type` value. Expected `by-date` or `by-range`." return None diff --git a/git_stacktrace/tests/test_git.py b/git_stacktrace/tests/test_git.py index e4ecf0a..4ca05cc 100644 --- a/git_stacktrace/tests/test_git.py +++ b/git_stacktrace/tests/test_git.py @@ -114,3 +114,15 @@ def test_pickaxe(self, mocked_command): "filename", ) mocked_command.assert_called_with(*expected) + + +class TestValidateGitRevisionArg(base.TestCase): + def test_accepts_normal_range_and_branch(self): + git.validate_git_revision_arg("abc..def", "git range") + git.validate_git_revision_arg("origin/master", "branch") + git.validate_git_revision_arg("", "branch") + git.validate_git_revision_arg(None, "branch") + + def test_rejects_leading_dash(self): + self.assertRaises(ValueError, git.validate_git_revision_arg, "--output=/tmp/x", "git range") + self.assertRaises(ValueError, git.validate_git_revision_arg, "-S", "branch") diff --git a/git_stacktrace/tests/test_server.py b/git_stacktrace/tests/test_server.py index d56a977..9ce1923 100644 --- a/git_stacktrace/tests/test_server.py +++ b/git_stacktrace/tests/test_server.py @@ -83,3 +83,15 @@ def test_args_byRange_returns_none_for_good_range(self, mock_valid_range): mock_valid_range.return_value = True args = Args({"option-type": "by-range"}) self.assertIsNone(args.validate()) + + def test_args_byRange_rejects_option_injection(self): + args = Args({"option-type": "by-range", "range": "--output=/tmp/sensitive.txt"}) + message = args.validate() + self.assertIn("must not start with '-'", message) + + def test_args_byDate_rejects_branch_option_injection(self): + args = Args( + {"option-type": "by-date", "since": "1.day", "branch": "--output=/tmp/sensitive.txt"} + ) + message = args.validate() + self.assertIn("must not start with '-'", message)