Reject git option injection in server mode

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:

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,13 +203,16 @@ 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()
     return len(lines) > 0
 
 
 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()

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

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")

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)