Skip to content

Commit

Permalink
Code security and quality improvements.
Browse files Browse the repository at this point in the history
  • Loading branch information
Lennolium committed Sep 28, 2023
1 parent aab7f9a commit 572e402
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 56 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ Regarding swiftGuard is a security application and therefore security is of the
that it is secure and reliable for all users. I am grateful for any feedback regarding security issues and will do my best to
address them as quickly as possible. Please refer to the [security policy](https://github.com/Lennolium/swiftGuard/blob/main/.github/SECURITY.md) for more information.

Additionally, I let my code be checked by several code quality and security tools (Bandit, Black, Codacy, CodeQL, PMD CPD, Prospector, Pylint, Pysa, Pyre, Trivy, Radon).
Additionally, I let my code be checked by several code quality and security tools (Bandit, Black, Codacy, CodeQL, PMD CPD, Prospector, Pylint, Pysa, Pyre, Trivy and Radon).
The results can be found by clicking on the badges below. These routines are **no replacement for a manual code and security audit**, but they help to find errors and vulnerabilities.
Please note that the results of these tools are not always accurate and may contain false positives.

Expand Down
113 changes: 64 additions & 49 deletions src/swiftguard/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,9 @@ def shutdown():

# AppleScript: slower, but only way to shut down without sudo.
osascript_path = "/usr/bin/osascript"
sd_process = subprocess.run([
osascript_path,
"-e",
'tell app "System Events" to shut down'],
) # nosec
sd_process = subprocess.run(
[osascript_path, "-e", 'tell app "System Events" to shut down'],
) # nosec

# Check exit code of osascript for success.
if sd_process.returncode != 0:
Expand All @@ -78,11 +76,9 @@ def hibernate():

# Second method/try (AppleScript, slower).
osascript_path = "/usr/bin/osascript"
subprocess.run([
osascript_path,
"-e",
'tell app "System Events" to sleep'],
) # nosec
subprocess.run(
[osascript_path, "-e", 'tell app "System Events" to sleep'],
) # nosec

# Return to prevent multiple execution.
return
Expand All @@ -106,8 +102,9 @@ def log(svt, msg, verbose=False):
os.mkdir(os.path.dirname(LOG_FILE))

# Copy log file to log dir.
shutil.copy(os.path.join(APP_PATH, "install", "swiftguard.log"),
LOG_FILE)
shutil.copy(
os.path.join(APP_PATH, "install", "swiftguard.log"), LOG_FILE
)

except Exception as e:
# Print error and exit.
Expand Down Expand Up @@ -148,23 +145,28 @@ def log(svt, msg, verbose=False):

# Log current USB state
if verbose:

system_profiler_path = "/usr/sbin/system_profiler"

verbose_process = subprocess.run([system_profiler_path, "SPUSBDataType"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True) # nosec
verbose_process = subprocess.run(
[system_profiler_path, "SPUSBDataType"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
) # nosec

# Check exit code of fdesetup for success.
if verbose_process.returncode != 0:
contents += (f"------ Current state: ------\n"
f"Could not get current USB state! Error: "
f"{verbose_process.stdout.strip()}")
contents += (
f"------ Current state: ------\n"
f"Could not get current USB state! Error: "
f"{verbose_process.stdout.strip()}"
)

else:
contents += (f"------ Current state: ------\n"
f"{verbose_process.stdout.strip()}")
contents += (
f"------ Current state: ------\n"
f"{verbose_process.stdout.strip()}"
)

# Write log to file.
with open(LOG_FILE, "a+") as log:
Expand Down Expand Up @@ -214,13 +216,11 @@ def config_load(config):
else:
restore_needed = False


if restore_needed:
print("okkk restore")
else:
print("okkk no restore")


# Defaulting some values if incorrect or not set.
default_needed = False

Expand Down Expand Up @@ -308,16 +308,17 @@ def config_write(config):


def check_encryption():
# TODO: docstring
# macOS: Check if FileVault (fv) is enabled.
if CURRENT_PLATFORM == "DARWIN":

fv_command = ["/usr/bin/fdesetup", "isactive"]


fv_process = subprocess.run(fv_command,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True) # nosec
fv_process = subprocess.run(
fv_command,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
) # nosec

# Check exit code of fdesetup for success.
if fv_process.returncode != 0:
Expand Down Expand Up @@ -371,12 +372,17 @@ def startup():
# manually add swiftGuard to the list of apps with permissions in
# System Preferences -> Security & Privacy -> Privacy -> Automation.
osascript_path = "/usr/bin/osascript"
permission_automation = subprocess.call([osascript_path,
'-e', 'tell application "System '
'Events"',
'-e', 'keystroke ""',
'-e', 'end tell']
) # nosec
permission_automation = subprocess.call(
[
osascript_path,
"-e",
'tell application "System ' 'Events"',
"-e",
'keystroke ""',
"-e",
"end tell",
]
) # nosec

if permission_automation == 1:
# Log error.
Expand All @@ -403,8 +409,11 @@ def startup():
log(1, "FileVault is disabled. Sensitive data SHOULD be encrypted.")

else:
log(1, f"Could not determine encryption status of host system! Error: "
f"{enc_status}.")
log(
1,
f"Could not determine encryption status of host system! Error: "
f"{enc_status}.",
)

# If no config file exists, copy the default config file.
if not os.path.isfile(CONFIG_FILE):
Expand All @@ -414,8 +423,10 @@ def startup():
os.mkdir(os.path.dirname(CONFIG_FILE))

# Copy config file to config dir.
shutil.copy(os.path.join(APP_PATH, "install", "swiftguard.ini"),
CONFIG_FILE)
shutil.copy(
os.path.join(APP_PATH, "install", "swiftguard.ini"),
CONFIG_FILE,
)

except Exception as e:
# Log error.
Expand Down Expand Up @@ -744,18 +755,22 @@ def usb_devices():

# Run system_profiler and capture its output.
result = subprocess.run(
[system_profiler_path, "SPUSBDataType", "-xml",
"-detailLevel", "mini"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=False,
) # nosec
[
system_profiler_path,
"SPUSBDataType",
"-xml",
"-detailLevel",
"mini",
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=False,
) # nosec

if result.returncode != 0:
# Handle the error case.
res_stderr = result.stderr.decode("utf-8").strip()
log(1, f"Error while running system_profiler: "
f"{res_stderr}.")
log(1, f"Error while running system_profiler: " f"{res_stderr}.")
return []

# Load the XML output using plistlib.
Expand Down Expand Up @@ -863,4 +878,4 @@ def bt_devices():
:return: A list of bluetooth devices
"""

pass
return
13 changes: 7 additions & 6 deletions src/swiftguard/swiftguard.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@
from PySide6.QtWidgets import QApplication, QMenu, QMessageBox, QSystemTrayIcon

from helpers import config_load, config_write, log, startup, usb_devices
# pylint: disable=unused-import
# noinspection PyUnresolvedReferences
from resources import resources_rc
from resources import resources_rc # noqa: F401
from worker import Worker

# Constants.
Expand Down Expand Up @@ -161,7 +162,8 @@ def __init__(self, function, states, icon, checked, full_name=None):
self.entry.setToolTip(self.states[1].lstrip())

# Connect the entry to the function.
self.entry.triggered.connect(lambda: self.toggle())
# self.entry.triggered.connect(lambda: self.toggle())
self.entry.triggered.connect(partial(self.toggle))

def toggle(self):
"""
Expand Down Expand Up @@ -260,10 +262,9 @@ def __init__(self, name, exclusive, *entries):
self.submenu.addAction(entry.entry)

# Connect the entry to the exclusive toggling function.
# ...connect(partial(lambda entry: self.toggle_excl(entry), entry))
if self.exclusive:
entry.entry.triggered.connect(
partial(lambda entry: self.toggle_excl(entry), entry)
)
entry.entry.triggered.connect(partial(self.toggle_excl, entry))

def toggle_excl(self, entry_clicked):
"""
Expand Down Expand Up @@ -937,7 +938,7 @@ def create_tray_icon(self):
entry08 = ToggleEntry(
self.config_update,
["60 s", " 60 s"],
QIcon(self.resources["usb-connection"]),
QIcon(self.resources["checkmark"]),
self.config["User"]["delay"] == "60",
)

Expand Down

0 comments on commit 572e402

Please sign in to comment.