Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Added E8010 to avoid using `os.popen()` as it internally uses `subprocess.Popen` with `shell=True`
- Added E8011 to avoid using `shlex.quote()` on non-POSIX platforms.

## [1.1.0] - 2021-07-02

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pylint plugin that enforces some secure coding standards.
| C8008 | Avoid `assert` statements in production code |
| 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 |


## Pre-commit hook
Expand Down
31 changes: 31 additions & 0 deletions pylint_secure_coding_standard.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,25 @@

"""Main file for the pylint_secure_coding_standard plugin"""

import platform

import astroid

from pylint.checkers import BaseChecker
from pylint.interfaces import IAstroidChecker


# ==============================================================================
# Helper functions


def _is_posix():
"""Return True if the current system is POSIX-compatible."""
# NB: we could simply use `os.name` instead of `platform.system()`. However, that solution would be difficult to
# test using `mock` as a few modules (like `pytest`) actually use it internally...
return platform.system() in ('Linux', 'Darwin')


# ==============================================================================


Expand Down Expand Up @@ -171,6 +184,15 @@ def _is_jsonpickle_encode_call(node):
return False


def _is_shlex_quote_call(node):
return not _is_posix() and (
isinstance(node.func, astroid.Attribute)
and isinstance(node.func.expr, astroid.Name)
and node.func.expr.name == 'shlex'
and node.func.attrname == 'quote'
)


# ==============================================================================


Expand Down Expand Up @@ -235,6 +257,11 @@ class SecureCodingStandardChecker(BaseChecker):
'avoid-os-popen',
'Use of `os.popen()` should be avoided, as it internally uses `subprocess.Popen` with `shell=True`',
),
'E8011': (
'Avoid using `shlex.quote()` on non-POSIX platforms',
'avoid-shlex-quote-on-non-posix',
'Use of `shlex.quote()` should be avoided on non-POSIX platforms (such as Windows)',
),
}

options = {}
Expand Down Expand Up @@ -263,6 +290,8 @@ def visit_call(self, node):
self.add_message('replace-builtin-open', node=node)
elif isinstance(node.func, astroid.Name) and (node.func.name in ('eval', 'exec')):
self.add_message('avoid-eval-exec', node=node)
elif _is_shlex_quote_call(node):
self.add_message('avoid-shlex-quote-on-non-posix', node=node)

def visit_import(self, node):
"""
Expand All @@ -288,6 +317,8 @@ def visit_importfrom(self, node):
self.add_message('avoid-os-system', node=node)
elif node.modname == 'os' and [name for (name, _) in node.names if name == 'popen']:
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)

def visit_with(self, node):
"""
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ build-backend = "setuptools.build_meta"
minversion = '6.0'
addopts = '-pno:warnings'
testpaths = ['tests']
mock_traceback_monkeypatch = false

[tool.setuptools_scm]
write_to = 'VERSION.txt'
Expand Down
2 changes: 2 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ install_requires =

[options.extras_require]
test =
mock
pytest
pytest-cov
pytest-mock


[bdist_wheel]
Expand Down
70 changes: 70 additions & 0 deletions tests/shlex_quote_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# -*- 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 pytest

import pylint_secure_coding_standard as pylint_scs
import pylint.testutils


class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker

@pytest.mark.parametrize(
'platform, expected_success',
(
('Linux', True),
('Darwin', True),
('Java', False),
('Windows', False),
),
)
@pytest.mark.parametrize('s', ('from shlex import quote',))
def test_shlex_quote_importfrom(self, mocker, platform, expected_success, s):
mocker.patch('platform.system', lambda: platform)

node = astroid.extract_node(s + ' #@')
if expected_success:
self.checker.visit_importfrom(node)
else:
with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shlex-quote-on-non-posix', node=node)):
self.checker.visit_importfrom(node)

@pytest.mark.parametrize(
'platform, expected_success',
(
('Linux', True),
('Darwin', True),
('Java', False),
('Windows', False),
),
)
@pytest.mark.parametrize(
's',
(
'shlex.quote("ls -l")',
'shlex.quote(command_str)',
),
)
def test_shlex_call_quote(self, mocker, platform, expected_success, s):
mocker.patch('platform.system', lambda: platform)

node = astroid.extract_node(s + ' #@')
if expected_success:
self.checker.visit_call(node)
else:
with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shlex-quote-on-non-posix', node=node)):
self.checker.visit_call(node)