Changeset 249200 in webkit

Timestamp:

Aug 28, 2019, 8:45:20 AM (7 years ago)

Author:

Jonathan Bedard

Message:

results.webkit.org: Sanitize all commit arguments on upload
​https://bugs.webkit.org/show_bug.cgi?id=201189
<rdar://problem/54564837>

Reviewed by Aakash Jain.

• resultsdbpy/resultsdbpy/controller/commit.py:

(Commit.init): Only allow commits to be constructed with valid values.

• resultsdbpy/resultsdbpy/controller/commit_controller.py:

(CommitController.register): Strip potential API key.

• resultsdbpy/resultsdbpy/controller/commit_unittest.py:

(CommitUnittest.test_invalid): Test that commits which contain html inside the
repository_id, branch or commit id are rejected.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• resultsdbpy/resultsdbpy/controller/commit.py (modified) (3 diffs)
• resultsdbpy/resultsdbpy/controller/commit_controller.py (modified) (1 diff)
• resultsdbpy/resultsdbpy/controller/commit_unittest.py (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2019-08-28  Jonathan Bedard  <jbedard@apple.com>
+
+        results.webkit.org: Sanitize all commit arguments on upload
+        https://bugs.webkit.org/show_bug.cgi?id=201189
+        <rdar://problem/54564837>
+
+        Reviewed by Aakash Jain.
+
+        * resultsdbpy/resultsdbpy/controller/commit.py:
+        (Commit.__init__): Only allow commits to be constructed with valid values.
+        * resultsdbpy/resultsdbpy/controller/commit_controller.py:
+        (CommitController.register): Strip potential API key.
+        * resultsdbpy/resultsdbpy/controller/commit_unittest.py:
+        (CommitUnittest.test_invalid): Test that commits which contain html inside the
+        repository_id, branch or commit id are rejected.
+
 2019-08-02  Claudio Saavedra  <csaavedra@igalia.com>
 

trunk/Tools/resultsdbpy/resultsdbpy/controller/commit.py

@@ -23 +23 @@
 import calendar
 import json
+import re
 
 from datetime import datetime
@@ -30 +31 @@
 class Commit(object):
     TIMESTAMP_TO_UUID_MULTIPLIER = 100
+    MAX_KEY_LENGTH = 128
 
     @classmethod
@@ -50 +52 @@
 
         self.repository_id = str(repository_id)
+        if not re.match(r'^[a-zA-Z?]+$', self.repository_id) or len(self.repository_id) > self.MAX_KEY_LENGTH:
+            raise ValueError(f"'{self.repository_id}' is an invalid repository id")
+
         self.branch = str(branch)
+        if not re.match(r'^[a-zA-Z0-9-.?/]+$', self.branch) or len(self.branch) > self.MAX_KEY_LENGTH:
+            raise ValueError(f"'{self.branch}' is an invalid branch name")
 
         # An id is either a git commit or SVN revision.
         self.id = str(id)
+        if not re.match(r'^[a-fA-F0-9?]+$', self.id) or len(self.id) > 40:
+            raise ValueError(f"'{self.id}' is an invalid commit id")
 
         if isinstance(timestamp, datetime):

trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_controller.py

@@ -268 +268 @@
             try:
                 commit = request.form or json.loads(request.get_data())
+                if 'api_key' in commit:
+                    del commit['api_key']
             except ValueError:
                 abort(400, description='Expected uploaded data to be json')

trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_unittest.py

@@ -101 +101 @@
 
     def test_invalid(self):
-        with self.assertRaises(ValueError):
+        with self.assertRaises(ValueError) as error:
             Commit(
                 repository_id='safari', branch='master',
@@ -107 +107 @@
                 timestamp=None,
             )
+        self.assertEqual(str(error.exception), 'timestamp is not defined for commit')
+
+        with self.assertRaises(ValueError) as error:
+            Commit(
+                repository_id='invalid-repo', branch='master',
+                id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+                timestamp=1537550685,
+            )
+        self.assertEqual(str(error.exception), "'invalid-repo' is an invalid repository id")
+
+        with self.assertRaises(ValueError) as error:
+            Commit(
+                repository_id='i' * 129, branch='master',
+                id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+                timestamp=1537550685,
+            )
+        self.assertEqual(str(error.exception), f"'{'i' * 129}' is an invalid repository id")
+
+        with self.assertRaises(ValueError) as error:
+            Commit(
+                repository_id='safari', branch='<html>invalid-branch</html>',
+                id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+                timestamp=1537550685,
+            )
+        self.assertEqual(str(error.exception), "'<html>invalid-branch</html>' is an invalid branch name")
+
+        with self.assertRaises(ValueError) as error:
+            Commit(
+                repository_id='safari', branch='i' * 129,
+                id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+                timestamp=1537550685,
+            )
+        self.assertEqual(str(error.exception), f"'{'i' * 129}' is an invalid branch name")
+
+        with self.assertRaises(ValueError) as error:
+            Commit(
+                repository_id='safari', branch='master',
+                id='<html>1234</html>',
+                timestamp=1537550685,
+            )
+        self.assertEqual(str(error.exception), "'<html>1234</html>' is an invalid commit id")
+
+        with self.assertRaises(ValueError) as error:
+            Commit(
+                repository_id='safari', branch='master',
+                id='0' * 41,
+                timestamp=1537550685,
+            )
+        self.assertEqual(str(error.exception), f"'{'0' * 41}' is an invalid commit id")