Skip to content

Commit

Permalink
Fix various bandit complaints. (#10)
Browse files Browse the repository at this point in the history
I ignore some, and fix others.
Full report before these changes:

```
$ bandit -r .
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.10.9
Run started:2023-01-12 21:41:24.463019

Test results:
>> Issue: [B404:blacklist] Consider possible security implications associated with the subprocess module.
   Severity: Low   Confidence: High
   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)
   Location: ./locales/update.py:5:0
   More Info: https://bandit.readthedocs.io/en/1.7.4/blacklists/blacklist_imports.html#b404-import-subprocess
4	import pkg_resources
5	import subprocess
6
7
8	domain = "pas.plugins.oidc"

--------------------------------------------------
>> Issue: [B105:hardcoded_password_string] Possible hardcoded password: ''
   Severity: Low   Confidence: Medium
   CWE: CWE-259 (https://cwe.mitre.org/data/definitions/259.html)
   Location: ./plugins.py:50:20
   More Info: https://bandit.readthedocs.io/en/1.7.4/plugins/b105_hardcoded_password_string.html
49	    client_id = ""
50	    client_secret = ""
51	    redirect_uris = ()

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   Location: ./plugins.py:105:8
   More Info: https://bandit.readthedocs.io/en/1.7.4/plugins/b101_assert_used.html
104	    def rememberIdentity(self, userinfo):
105	        assert isinstance(userinfo, OpenIDSchema)
106	        # sub: machine-readable identifier of the user at this server;
107	        #      this value is guaranteed to be unique per user, stable over time,
108	        #      and never re-used
109	        user_id = userinfo["sub"]

--------------------------------------------------
>> Issue: [B110:try_except_pass] Try, Except, Pass detected.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   Location: ./plugins.py:147:28
   More Info: https://bandit.readthedocs.io/en/1.7.4/plugins/b110_try_except_pass.html
146	                                raise
147	                            except Exception:
148	                                pass

--------------------------------------------------
>> Issue: [B311:blacklist] Standard pseudo-random generators are not suitable for security/cryptographic purposes.
   Severity: Low   Confidence: High
   CWE: CWE-330 (https://cwe.mitre.org/data/definitions/330.html)
   Location: ./plugins.py:185:24
   More Info: https://bandit.readthedocs.io/en/1.7.4/blacklists/blacklist_calls.html#b311-random
184	        """Return a obfuscated password never used for login"""
185	        return "".join([choice(PWCHARS) for ii in range(40)])
186

--------------------------------------------------

Code scanned:
	Total lines of code: 653
	Total lines skipped (#nosec): 3

Run metrics:
	Total issues (by severity):
		Undefined: 0
		Low: 5
		Medium: 0
		High: 0
	Total issues (by confidence):
		Undefined: 0
		Low: 0
		Medium: 1
		High: 4
Files skipped (0):
```
  • Loading branch information
mauritsvanrees committed Jan 13, 2023
1 parent aa9f42c commit 79d3f32
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/pas/plugins/oidc/locales/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import os
import pkg_resources
import subprocess
import subprocess # nosec B404


domain = "pas.plugins.oidc"
Expand Down
22 changes: 17 additions & 5 deletions src/pas/plugins/oidc/plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from Products.PluggableAuthService.interfaces.plugins import IUserAdderPlugin
from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin
from Products.PluggableAuthService.utils import classImplements
from random import choice
from ZODB.POSException import ConflictError
from zope.interface import implementer
from zope.interface import Interface
Expand All @@ -27,6 +26,14 @@
import logging
import string

try:
# Python 3.6+
from secrets import choice
except ImportError:
# Less secure.
# https://bandit.readthedocs.io/en/1.7.4/blacklists/blacklist_calls.html#b311-random
from random import choice


logger = logging.getLogger(__name__)
# _MARKER = object()
Expand All @@ -47,7 +54,7 @@ class OIDCPlugin(BasePlugin):

issuer = ""
client_id = ""
client_secret = ""
client_secret = "" # nosec B105
redirect_uris = ()
use_session_data_manager = False
create_ticket = True
Expand Down Expand Up @@ -102,7 +109,8 @@ class OIDCPlugin(BasePlugin):
)

def rememberIdentity(self, userinfo):
assert isinstance(userinfo, OpenIDSchema)
if not isinstance(userinfo, OpenIDSchema):
raise AssertionError("userinfo should be an OpenIDSchema but is {}".format(type(userinfo)))
# sub: machine-readable identifier of the user at this server;
# this value is guaranteed to be unique per user, stable over time,
# and never re-used
Expand Down Expand Up @@ -144,7 +152,11 @@ def rememberIdentity(self, userinfo):
membershipTool.createMemberArea(user_id)
except (ConflictError, KeyboardInterrupt):
raise
except Exception:
except Exception: # nosec B110
# Silently ignored exception, but seems fine here.
# Logging would likely generate too much noise,
# depending on your setup.
# https://bandit.readthedocs.io/en/1.7.4/plugins/b110_try_except_pass.html
pass
self._updateUserProperties(user, userinfo)
break
Expand Down Expand Up @@ -182,7 +194,7 @@ def _updateUserProperties(self, user, userinfo):

def _generatePassword(self):
"""Return a obfuscated password never used for login"""
return "".join([choice(PWCHARS) for ii in range(40)])
return "".join([choice(PWCHARS) for ii in range(40)]) # nosec B311

def _setupTicket(self, user_id):
"""Set up authentication ticket (__ac cookie) with plone.session.
Expand Down

0 comments on commit 79d3f32

Please sign in to comment.