Add E8013, E8014 and E8015 + fix test names

Takishima · Takishima · commit 09cf3af1591f · 2021-07-26T17:38:54.000+02:00
- E8013: avoid `pickle.load()` and `pickle.loads()`
- E8014: avoid `marshal.load()` and `marshal.loads()`
- E8015: avoid `shelve.open()`
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 -   Add plugin option to control whether we favour `os.open` over the builtin `open`
 -   Added W8012 to warn when using `os.open` with unsafe permissions
+-   Added E8013 to avoid using `pickle.load` and `pickle.loads`
+-   Added E8014 to avoid using `marshal.load` and `marshal.loads`
+-   Added E8015 to avoid using `shelve.open`
+
+### Fixed
+
+-   Fixed a few test function names
 
 ### Repository
 
diff --git a/README.md b/README.md
@@ -25,7 +25,10 @@ pylint plugin that enforces some secure coding standards.
 | R8009 | Use of builtin `open` for writing is discouraged in favor of `os.open` to allow for setting file permissions |
 | E8010 | Avoid using `os.popen()` as it internally uses `subprocess.Popen` with `shell=True`                          |
 | E8011 | Use of `shlex.quote()` should be avoided on non-POSIX platforms                                              |
-| W8012 | Avoid using `os.open` with unsafe permissions permissions                                                    |
+| W8012 | Avoid using `os.open()` with unsafe permissions permissions                                                  |
+| E8013 | Avoid using `pickle.load()` and `pickle.loads()`                                                             |
+| E8014 | Avoid using `marshal.load()` and `marshal.loads()`                                                           |
+| E8015 | Avoid using `shelve.open()`                                                                                  |
 
 
 ## Plugin configuration options
diff --git a/pylint_secure_coding_standard.py b/pylint_secure_coding_standard.py
@@ -36,11 +36,13 @@ def _is_posix():
 
 
 def _is_function_call(node, module, function):
+    if not isinstance(function, (list, tuple)):
+        function = (function,)
     return (
         isinstance(node.func, astroid.Attribute)
         and isinstance(node.func.expr, astroid.Name)
         and node.func.expr.name == module
-        and node.func.attrname == function
+        and node.func.attrname in function
     )
 
 
@@ -314,6 +316,21 @@ class SecureCodingStandardChecker(BaseChecker):
             'os-open-unsafe-permissions',
             'Avoid using `os.open` with unsafe file permissions (by default 0 <= mode <= 0o755)',
         ),
+        'E8013': (
+            'Avoid using `pickle.load` and `pickle.loads`',
+            'avoid-pickle-load',
+            'Use of `pickle.load` and `pickle.loads` should be avoided in favour of safer file formats',
+        ),
+        'E8014': (
+            'Avoid using `marshal.load` and `marshal.loads`',
+            'avoid-marshal-load',
+            'Use of `marshal.load` and `marshal.loads` should be avoided in favour of safer file formats',
+        ),
+        'E8015': (
+            'Avoid using `shelve.open()`',
+            'avoid-shelve-open',
+            'Use of `shelve.open()` should be avoided in favour of safer file formats',
+        ),
     }
 
     def __init__(self, *args, **kwargs):
@@ -322,7 +339,7 @@ def __init__(self, *args, **kwargs):
         self._prefer_os_open = False
         self._os_open_modes_allowed = []
 
-    def visit_call(self, node):
+    def visit_call(self, node):  # pylint: disable=too-many-branches
         """Visitor method called for astroid.Call nodes."""
         if _is_pdb_call(node):
             self.add_message('avoid-debug-stmt', node=node)
@@ -352,6 +369,12 @@ def visit_call(self, node):
             and not _is_os_open_allowed_mode(node, self._os_open_modes_allowed)
         ):
             self.add_message('os-open-unsafe-permissions', node=node)
+        elif _is_function_call(node, module='pickle', function=('load', 'loads')):
+            self.add_message('avoid-pickle-load', node=node)
+        elif _is_function_call(node, module='marshal', function=('load', 'loads')):
+            self.add_message('avoid-marshal-load', node=node)
+        elif _is_function_call(node, module='shelve', function='open'):
+            self.add_message('avoid-shelve-open', node=node)
 
     def visit_import(self, node):
         """Visitor method called for astroid.Import nodes."""
@@ -381,18 +404,26 @@ def visit_importfrom(self, node):
             self.add_message('avoid-os-popen', node=node)
         elif not _is_posix() and node.modname == 'shlex' and [name for (name, _) in node.names if name == 'quote']:
             self.add_message('avoid-shlex-quote-on-non-posix', node=node)
+        elif node.modname == 'pickle' and [name for (name, _) in node.names if name in ('load', 'loads')]:
+            self.add_message('avoid-pickle-load', node=node)
+        elif node.modname == 'marshal' and [name for (name, _) in node.names if name in ('load', 'loads')]:
+            self.add_message('avoid-marshal-load', node=node)
+        elif node.modname == 'shelve' and [name for (name, _) in node.names if name == 'open']:
+            self.add_message('avoid-shelve-open', node=node)
 
     def visit_with(self, node):
         """Visitor method called for astroid.With nodes."""
-        if self._prefer_os_open:
-            for item in node.items:
-                if item and isinstance(item[0], astroid.Call):
+        for item in node.items:
+            if item and isinstance(item[0], astroid.Call):
+                if self._prefer_os_open:
                     if _is_builtin_open_for_writing(item[0]):
                         self.add_message('replace-builtin-open', node=node)
                     elif _is_function_call(item[0], module='os', function='open') and not _is_os_open_allowed_mode(
                         item[0], self._os_open_modes_allowed
                     ):
                         self.add_message('os-open-unsafe-permissions', node=node)
+                elif _is_function_call(item[0], module='shelve', function='open'):
+                    self.add_message('avoid-shelve-open', node=node)
 
     def visit_assert(self, node):
         """Visitor method called for astroid.Assert nodes."""
diff --git a/tests/jsonpickle_decode_test.py b/tests/jsonpickle_decode_test.py
@@ -23,7 +23,7 @@
 class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
     CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
 
-    def test_yaml_ok(self):
+    def test_jsonpickle_decode_ok(self):
         nodes = astroid.extract_node(
             """
             int(0) #@
@@ -41,7 +41,7 @@ def test_yaml_ok(self):
         's',
         ('jsonpickle.decode(payload)',),
     )
-    def test_yaml_not_ok(self, s):
+    def test_jsonpickle_decode_not_ok(self, s):
         node = astroid.extract_node(s + ' #@')
         with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-jsonpickle-decode', node=node)):
             self.checker.visit_call(node)
diff --git a/tests/marshal_load_test.py b/tests/marshal_load_test.py
@@ -0,0 +1,64 @@
+# -*- coding: utf-8 -*-
+# Copyright 2021 Damien Nguyen
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import astroid
+import pylint.testutils
+import pytest
+
+import pylint_secure_coding_standard as pylint_scs
+
+
+class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
+    CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
+
+    def test_marshal_load_ok(self):
+        nodes = astroid.extract_node(
+            """
+            int(0) #@
+            foo() #@
+            marshal.dump(data, "file.txt") #@
+            marshal.dumps(data) #@
+            """
+        )
+
+        with self.assertNoMessages():
+            for node in nodes:
+                self.checker.visit_call(node)
+
+    @pytest.mark.parametrize(
+        's',
+        (
+            'marshal.load("file.txt")',
+            r'marshal.loads(b"\xe9\x01\x00\x00\x00")',
+            'marshal.loads(data)',
+        ),
+    )
+    def test_marshal_load_not_ok(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-marshal-load', node=node)):
+            self.checker.visit_call(node)
+
+    @pytest.mark.parametrize(
+        's',
+        (
+            'from marshal import load',
+            'from marshal import loads',
+            'from marshal import dump, load',
+        ),
+    )
+    def test_marshal_open_importfrom(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-marshal-load', node=node)):
+            self.checker.visit_importfrom(node)
diff --git a/tests/pickle_load_test.py b/tests/pickle_load_test.py
@@ -0,0 +1,64 @@
+# -*- coding: utf-8 -*-
+# Copyright 2021 Damien Nguyen
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import astroid
+import pylint.testutils
+import pytest
+
+import pylint_secure_coding_standard as pylint_scs
+
+
+class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
+    CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
+
+    def test_pickle_load_ok(self):
+        nodes = astroid.extract_node(
+            """
+            int(0) #@
+            foo() #@
+            pickle.dump(data, "file.txt") #@
+            pickle.dumps(data) #@
+            """
+        )
+
+        with self.assertNoMessages():
+            for node in nodes:
+                self.checker.visit_call(node)
+
+    @pytest.mark.parametrize(
+        's',
+        (
+            'pickle.load("file.txt")',
+            r'pickle.loads(b"\x80\x04K\x01.")',
+            'pickle.loads(data)',
+        ),
+    )
+    def test_pickle_load_not_ok(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-pickle-load', node=node)):
+            self.checker.visit_call(node)
+
+    @pytest.mark.parametrize(
+        's',
+        (
+            'from pickle import load',
+            'from pickle import loads',
+            'from pickle import dump, load',
+        ),
+    )
+    def test_pickle_open_importfrom(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-pickle-load', node=node)):
+            self.checker.visit_importfrom(node)
diff --git a/tests/shelve_open_test.py b/tests/shelve_open_test.py
@@ -0,0 +1,59 @@
+# -*- coding: utf-8 -*-
+# Copyright 2021 Damien Nguyen
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import astroid
+import pylint.testutils
+import pytest
+
+import pylint_secure_coding_standard as pylint_scs
+
+
+class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
+    CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
+
+    def test_shelve_open_ok(self):
+        nodes = astroid.extract_node(
+            """
+            int(0) #@
+            foo() #@
+            """
+        )
+
+        with self.assertNoMessages():
+            for node in nodes:
+                self.checker.visit_call(node)
+
+    _not_ok = (
+        'shelve.open("file.txt")',
+        'shelve.open(filename)',
+    )
+
+    @pytest.mark.parametrize('s', _not_ok)
+    def test_shelve_open_call(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shelve-open', node=node)):
+            self.checker.visit_call(node)
+
+    @pytest.mark.parametrize('s', _not_ok)
+    def test_shelve_open_with(self, s):
+        node = astroid.extract_node(f'with {s} as fd: fd.read() #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shelve-open', node=node)):
+            self.checker.visit_with(node)
+
+    @pytest.mark.parametrize('s', ('from shelve import open',))
+    def test_shelve_open_importfrom(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shelve-open', node=node)):
+            self.checker.visit_importfrom(node)
