Modify check-webkit-style to prohibit sensitive phrases
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Oct 2016 16:38:33 +0000 (16:38 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Oct 2016 16:38:33 +0000 (16:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163048
<rdar://problem/28289755>

Terms considered or found to be too general to flag:
ASSERT_WITH_SECURITY_IMPLICATION, bad cast, bug, bypass, crash,
denial of service, dereference, disclosure, error, exploit,
failure, heap, integer overflow, leak, null dereference,
null pointer dereference, out of bounds, overflow,
race condition, sensitive information, stack, type confusion.

Reviewed by Brent Fulgham.

* Scripts/webkitpy/style/checkers/changelog.py:
(ChangeLogChecker.check_entry):
    Now calls ChangeLogChecker.check_for_unwanted_security_terms().
(ChangeLogChecker):
(ChangeLogChecker.check_for_unwanted_security_terms):
    New function to check for sensitive terms.
(ChangeLogChecker.check_for_unwanted_security_terms.FoundUnwantedSecurityTerm):
(ChangeLogChecker.check_for_unwanted_security_terms.FoundUnwantedSecurityTerm.__init__):
    Convenience class.
* Scripts/webkitpy/style/checkers/changelog_unittest.py:
(ChangeLogCheckerTest.test_unwanted_security_terms):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207146 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/style/checkers/changelog.py
Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py

index 884276a..f671270 100644 (file)
@@ -1,3 +1,30 @@
+2016-10-11  John Wilander  <wilander@apple.com>
+
+        Modify check-webkit-style to prohibit sensitive phrases
+        https://bugs.webkit.org/show_bug.cgi?id=163048
+        <rdar://problem/28289755>
+
+        Terms considered or found to be too general to flag:
+        ASSERT_WITH_SECURITY_IMPLICATION, bad cast, bug, bypass, crash,
+        denial of service, dereference, disclosure, error, exploit,
+        failure, heap, integer overflow, leak, null dereference,
+        null pointer dereference, out of bounds, overflow,
+        race condition, sensitive information, stack, type confusion.
+
+        Reviewed by Brent Fulgham.
+
+        * Scripts/webkitpy/style/checkers/changelog.py:
+        (ChangeLogChecker.check_entry):
+            Now calls ChangeLogChecker.check_for_unwanted_security_terms().
+        (ChangeLogChecker):
+        (ChangeLogChecker.check_for_unwanted_security_terms):
+            New function to check for sensitive terms.
+        (ChangeLogChecker.check_for_unwanted_security_terms.FoundUnwantedSecurityTerm):
+        (ChangeLogChecker.check_for_unwanted_security_terms.FoundUnwantedSecurityTerm.__init__):
+            Convenience class.
+        * Scripts/webkitpy/style/checkers/changelog_unittest.py:
+        (ChangeLogCheckerTest.test_unwanted_security_terms):
+
 2016-10-11  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r207067.
index dce85af..f31666a 100644 (file)
@@ -24,6 +24,7 @@
 """Checks WebKit style for ChangeLog files."""
 
 from common import TabChecker, match, search, searchIgnorecase
+from sys import maxsize
 from webkitpy.common.checkout.changelog import parse_bug_id_from_changelog
 
 
@@ -74,6 +75,8 @@ class ChangeLogChecker(object):
                                         "changelog/nonewtests", 5,
                                         "You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.")
 
+        self.check_for_unwanted_security_phrases(first_line_checked, entry_lines)
+
     def check(self, lines):
         self._tab_checker.check(lines)
         first_line_checked = 0
@@ -91,3 +94,36 @@ class ChangeLogChecker(object):
             entry_lines.append(line)
 
         self.check_entry(first_line_checked, entry_lines)
+
+    def contains_phrase_in_first_line_or_across_two_lines(self, phrase, line1, line2):
+        return searchIgnorecase(phrase, line1) or ((not searchIgnorecase(phrase, line2)) and searchIgnorecase(phrase, line1 + " " + line2))
+
+    def check_for_unwanted_security_phrases(self, first_line_checked, lines):
+        unwanted_security_phrases = [
+            "arbitrary code execution", "buffer overflow", "buffer overrun",
+            "buffer underrun", "dangling pointer", "double free", "fuzzer", "fuzzing", "fuzz test",
+            "invalid cast", "jsfunfuzz", "malicious", "memory corruption", "security bug",
+            "security flaw", "use after free", "use-after-free", "UXSS",
+            "WTFCrashWithSecurityImplication",
+            "spoof",  # Captures spoof, spoofed, spoofing
+            "vulnerab",  # Captures vulnerable, vulnerability, vulnerabilities
+        ]
+
+        lines_with_single_spaces = []
+        for line in lines:
+            lines_with_single_spaces.append(" ".join(line.split()))
+
+        found_unwanted_security_phrases = []
+        last_index = len(lines_with_single_spaces) - 1
+        first_line_number_with_unwanted_phrase = maxsize
+        for unwanted_phrase in unwanted_security_phrases:
+            for line_index, line in enumerate(lines_with_single_spaces):
+                next_line = "" if line_index >= last_index else lines_with_single_spaces[line_index + 1]
+                if self.contains_phrase_in_first_line_or_across_two_lines(unwanted_phrase, line, next_line):
+                    found_unwanted_security_phrases.append(unwanted_phrase)
+                    first_line_number_with_unwanted_phrase = min(first_line_number_with_unwanted_phrase, first_line_checked + line_index)
+
+        if len(found_unwanted_security_phrases) > 0:
+            self.handle_style_error(first_line_number_with_unwanted_phrase,
+                                    "changelog/unwantedsecurityterms", 3,
+                                    "Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: {}".format(", ".join(found_unwanted_security_phrases)))
index 8df43b0..d09f961 100644 (file)
@@ -180,3 +180,66 @@ class ChangeLogCheckerTest(unittest.TestCase):
                              '        * Source/WebKit/foo.cpp:    \n'
                              '        * Source/WebKit/bar.cpp:\n'
                              '        * Source/WebKit/foobar.cpp: Description\n')
+
+    def test_unwanted_security_terms(self):
+        self.assert_error(5, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        ExampleBug\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        A buffer overflow existed in code.\n')
+        self.assert_error(9, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        ExampleBug\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        This patch addresses a great number of issues.\n'
+                          '        Therefore there is a lot to say here about a great\n'
+                          '        many things such as the weather, the latest and\n'
+                          '        greatest in sports, and the mood of fiction\n'
+                          '        characters. Anyway the patch fixes a use after\n'
+                          '        free which is not good. Or rather, it is good\n'
+                          '        that it is fixed but not good that it existed.\n')
+        self.assert_error(5, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        ExampleBug\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        This patch addresses a pretty bad buffer\n'
+                          '        overflow in\n')
+        self.assert_error(2, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        Fix use after free\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        A good fix.\n')
+        self.assert_error(5, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        ExampleBug\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        Bug found through fuzzing.\n')
+        self.assert_error(11, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        ExampleBug\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        Bug found through testing.\n'
+                          '\n'
+                          '        Several new tests added.\n'
+                          '\n'
+                          '        * Source/WebKit/foo.cpp:    \n'
+                          '        * Source/WebKit/bar.cpp:\n'
+                          '        * Source/WebKit/foobar.cpp: Vulnerabilities fixed\n')
+        self.assert_error(5, range(1, 20), 'changelog/unwantedsecurityterms',
+                          '2016-11-11 Bogus Person <bperson@example.com>\n'
+                          '        ExampleBug with several security sensitive terms in change log\n'
+                          '        http://bugs.webkit.org/show_bug.cgi?id=12345\n'
+                          '\n'
+                          '        Use-after-free found through testing.\n'
+                          '\n'
+                          '        Several new tests added to check double free.\n'
+                          '\n'
+                          '        * Source/WebKit/foo.cpp:    \n'
+                          '        * Source/WebKit/bar.cpp:\n'
+                          '        * Source/WebKit/foobar.cpp: memory CORRUPTION fixed\n')