search

lnls-sirius / pydm-opi

PyDM based interfaces used in various applications.
GNU General Public License v3.0
2 stars 4 forks source link

beaglebone: Unwanted file #68

Closed carneirofc closed 3 years ago

carneirofc commented 3 years ago

@ntanck I have noticed that the file https://github.com/lnls-sirius/pydm-opi/blob/master/src/siriushlacon/beaglebones/bbb.py should be present on the beaglebone only, and it was probably included by mistake. Can I safely delete it? There are arguably dangerous function calls and to be extra safe we should not include them on this repo https://github.com/lnls-sirius/pydm-opi/blob/master/src/siriushlacon/beaglebones/bbb.py#L88.

I plan to use bandit regularly and this script is a bit problematic.

carneirofc commented 3 years ago

bandit output, for informational purposes only:

[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.6.13
Run started:2021-05-14 18:54:57.761717

Test results:
>> Issue: [B403:blacklist] Consider possible security implications associated with pickle module.
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:3
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_imports.html#b403-import-pickle
2       import logging
3       import pickle
4       import os

--------------------------------------------------
>> Issue: [B404:blacklist] Consider possible security implications associated with subprocess module.
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:5
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_imports.html#b404-import-subprocess
4       import os
5       import subprocess
6       import time

--------------------------------------------------
>> Issue: [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
   Severity: Medium   Confidence: Medium
   Location: .\src\siriushlacon\beaglebones\bbb.py:22
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b108_hardcoded_tmp_directory.html
21
22          def __init__(self, path="/var/tmp/bbb.bin", interface="eth0"):
23              """

--------------------------------------------------
>> Issue: [B605:start_process_with_a_shell] Starting a process with a shell: Seems safe, but may be changed in the future, consider rewriting without shell
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:88
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b605_start_process_with_a_shell.html
87              self.logger.info("Rebooting system.")
88              os.system("reboot")
89

--------------------------------------------------
>> Issue: [B607:start_process_with_partial_path] Starting a process with a partial executable path
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:88
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html
87              self.logger.info("Rebooting system.")
88              os.system("reboot")
89

--------------------------------------------------
>> Issue: [B605:start_process_with_a_shell] Starting a process with a shell, possible injection detected, security issue.
   Severity: High   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:107
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b605_start_process_with_a_shell.html
106                     hostnameFile.close()
107                 os.system("hostname {}".format(new_hostname))
108                 self.node.name = new_hostname

--------------------------------------------------
>> Issue: [B607:start_process_with_partial_path] Starting a process with a partial executable path
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:141
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html
140
141             name = subprocess.check_output(["hostname"]).decode("utf-8").strip("\n")
142             self.node.name = name.replace("--", ":")

--------------------------------------------------
>> Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:141
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b603_subprocess_without_shell_equals_true.html
140
141             name = subprocess.check_output(["hostname"]).decode("utf-8").strip("\n")
142             self.node.name = name.replace("--", ":")

--------------------------------------------------
>> Issue: [B301:blacklist] Pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue.
   Severity: Medium   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:149
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b301-pickle
148             with open(self.configuration_file_path, "rb") as file:
149                 self.node = pickle.load(file)
150                 file.close()

--------------------------------------------------
>> Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:167
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b603_subprocess_without_shell_equals_true.html
166             """
167             command_out = subprocess.check_output(
168                 "ip addr show dev {} scope global".format(self.interface_name).split()
169             ).decode("utf-8")

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:216
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
215                 ],
216                 shell=True,
217             )
218
219             time.sleep(2)
220             self.logger.debug(
221                 "IP address after update is {}".format(self.get_ip_address()[0])
222             )
223

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:235
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
234                 ],
235                 shell=True,
236             )
237
238         def get_connman_service_name(self):
239             """
240             Returns the service name assigned to manage an interface.
241             @fixme: services with spaces on their names won't be detected!
242             :return: A service's name.

--------------------------------------------------
>> Issue: [B607:start_process_with_partial_path] Starting a process with a partial executable path
   Severity: Low   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:245
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html
244             """
245             services = subprocess.check_output(
246                 ["connmanctl services"], stderr=subprocess.STDOUT, shell=True
247             ).decode("utf-8")

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:246
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
245             services = subprocess.check_output(
246                 ["connmanctl services"], stderr=subprocess.STDOUT, shell=True
247             ).decode("utf-8")
248

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: .\src\siriushlacon\beaglebones\bbb.py:255
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
254                         stderr=subprocess.STDOUT,
255                         shell=True,
256                     ).decode("utf-8")
257
258                     for prop in service_properties.split("\n"):
259                         if prop.strip().startswith("Ethernet"):

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

Code scanned:
        Total lines of code: 533
        Total lines skipped (#nosec): 0

Run metrics:
        Total issues (by severity):
                Undefined: 0.0
                Low: 8.0
                Medium: 2.0
                High: 5.0
        Total issues (by confidence):
                Undefined: 0.0
                Low: 0.0
                Medium: 1.0
                High: 14.0
Files skipped (0):
gfrn commented 3 years ago

Yea, no problem at all. I've kept it mainly due to inertia from the original project, it has no purpose at all in the UI.

Also, I'm looking into fixing these security problems as well, along with some refactoring

carneirofc commented 3 years ago

Nice, I will be removing this file soon. Regarding these warnings, bandit has been really useful. Although I am deliberately ignoring some warnings.

gfrn commented 3 years ago

I think I'm going to start using it for some other projects as well, I've been looking into it and it seems like a pretty nice tool. Some warnings really need to be ignored (like Flake8 and how to this day it still gets annoyed at E203/W503). Thanks for the heads up on the warnings!