From 2497a389354354ab7b61f884ce8a8e9dd16fe877 Mon Sep 17 00:00:00 2001
From: Eric Brown <browne@vmware.com>
Date: Thu, 20 Sep 2018 17:35:19 -0700
Subject: [PATCH] Properly handle nosec strings in code

The string "# nosec" has a chance (albeit remote) that it can be
part of the code on the line not just within the comment. Bandit
currently checks the entire line for that string.

This patch tokenizes the line of input to figure out what piece
is a comment, then compares whether that comment contains "# nosec".

Fixes #383

Signed-off-by: Eric Brown <browne@vmware.com>
---
 bandit/core/manager.py              | 19 +++++++++++++++----
 examples/nosec.py                   |  2 ++
 pylintrc                            |  3 ++-
 tests/functional/test_functional.py |  4 ++--
 tests/functional/test_runtime.py    | 11 +++++++----
 5 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/bandit/core/manager.py b/bandit/core/manager.py
index 48e80eb68..0352a96a5 100644
--- a/bandit/core/manager.py
+++ b/bandit/core/manager.py
@@ -20,8 +20,11 @@
 import logging
 import os
 import sys
+import tokenize
 import traceback
 
+import six
+
 from bandit.core import constants as b_constants
 from bandit.core import extension_loader
 from bandit.core import issue
@@ -271,10 +274,18 @@ def _parse_file(self, fname, fdata, new_files_list):
             if self.ignore_nosec:
                 nosec_lines = set()
             else:
-                nosec_lines = set(
-                    lineno + 1 for
-                    (lineno, line) in enumerate(lines)
-                    if b'#nosec' in line or b'# nosec' in line)
+                try:
+                    fdata.seek(0)
+                    if six.PY2:
+                        tokens = tokenize.generate_tokens(fdata.readline)
+                    else:
+                        tokens = tokenize.tokenize(fdata.readline)
+                    nosec_lines = set(
+                        lineno for toktype, tokval, (lineno, _), _, _ in tokens
+                        if toktype == tokenize.COMMENT and
+                        '#nosec' in tokval or '# nosec' in tokval)
+                except tokenize.TokenError as e:
+                    nosec_lines = set()
             score = self._execute_ast_visitor(fname, data, nosec_lines)
             self.scores.append(score)
             self.metrics.count_issues([score, ])
diff --git a/examples/nosec.py b/examples/nosec.py
index fab06d28e..6bc2cc296 100644
--- a/examples/nosec.py
+++ b/examples/nosec.py
@@ -3,3 +3,5 @@
                  shell=True)
 subprocess.Popen('/bin/ls *',
                  shell=True)  #nosec (on the specific kwarg line)
+subprocess.Popen('#nosec', shell=True)
+subprocess.Popen('/bin/ls *', shell=True) # type: … # nosec # noqa: E501 ; pylint: disable=line-too-long
diff --git a/pylintrc b/pylintrc
index 366b29a7e..9296a8b38 100644
--- a/pylintrc
+++ b/pylintrc
@@ -12,6 +12,7 @@
 # TODO(browne): fix these in the future
 # C0103: invalid-name
 # E1101: no-member
+# E1111: assignment-from-no-return
 # R0204: redefined-variable-type
 # R0902: too-many-instance-attributes
 # R0912: too-many-branches
@@ -28,7 +29,7 @@
 # W0613: unused-argument
 # W0621: redefined-outer-name
 # W0703: broad-except
-disable=C0111,C0301,C0325,F0401,W0511,W0142,W0622,C0103,E1101,R0204,R0902,R0912,R0913,R0914,R0915,W0110,W0141,W0201,W0401,W0603,W0212,W0612,W0613,W0621,W0703
+disable=C0111,C0301,C0325,F0401,W0511,W0142,W0622,C0103,E1101,E1111,R0204,R0902,R0912,R0913,R0914,R0915,W0110,W0141,W0201,W0401,W0603,W0212,W0612,W0613,W0621,W0703
 
 [Basic]
 # Variable names can be 1 to 31 characters long, with lowercase and underscores
diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py
index 0a7af9619..85bf9cb5a 100644
--- a/tests/functional/test_functional.py
+++ b/tests/functional/test_functional.py
@@ -708,8 +708,8 @@ def test_flask_debug_true(self):
 
     def test_nosec(self):
         expect = {
-            'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 0},
-            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 0}
+            'SEVERITY': {'UNDEFINED': 0, 'LOW': 2, 'MEDIUM': 0, 'HIGH': 0},
+            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2}
         }
         self.check_example('nosec.py', expect)
 
diff --git a/tests/functional/test_runtime.py b/tests/functional/test_runtime.py
index a7f408172..d59a4113d 100644
--- a/tests/functional/test_runtime.py
+++ b/tests/functional/test_runtime.py
@@ -15,6 +15,7 @@
 import os
 import subprocess
 
+import six
 import testtools
 
 
@@ -111,11 +112,13 @@ def test_example_nonsense2(self):
             ['bandit', ], ['nonsense2.py', ]
         )
         self.assertEqual(0, retcode)
-        self.assertIn(
-            "Exception occurred when executing tests against", output
-        )
         self.assertIn("Files skipped (1):", output)
-        self.assertIn("nonsense2.py (exception while scanning file)", output)
+        if six.PY2:
+            self.assertIn("nonsense2.py (exception while scanning file)",
+                          output)
+        else:
+            self.assertIn("nonsense2.py (syntax error while parsing AST",
+                          output)
 
     def test_example_imports(self):
         (retcode, output) = self._test_example(['bandit', ], ['imports.py', ])