From cc3b2ec2026efdd27140519525d8453682674688 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 14 Jul 2021 12:22:45 -0700 Subject: [PATCH 1/4] fix(security): Improve sql in version_signature.py * improves version_signature.py to align with bandit's guidance * fixes #1229 Signed-off-by: Terri Oda --- cve_bin_tool/version_signature.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/cve_bin_tool/version_signature.py b/cve_bin_tool/version_signature.py index dcd280bac9..9b4ba432a6 100644 --- a/cve_bin_tool/version_signature.py +++ b/cve_bin_tool/version_signature.py @@ -54,9 +54,7 @@ def get_mapping_data(self): the data after the specified refresh duration """ self.cursor.execute( - "CREATE TABLE IF NOT EXISTS {}(version TEXT , sourceId TEXT PRIMARY KEY)".format( - self.table_name - ) + f"CREATE TABLE IF NOT EXISTS {self.table_name}(version TEXT , sourceId TEXT PRIMARY KEY)" ) self.cursor.execute( @@ -81,23 +79,22 @@ def get_mapping_data(self): if datestamp is None or update_required: # if update is required or database is empty, fetch and insert data into database - self.cursor.execute(f"DELETE FROM {self.table_name}") - self.cursor.execute( - "DELETE FROM {}".format("latest_update_" + self.table_name) - ) + self.cursor.execute("DELETE FROM ?", self.table_name) + self.cursor.execute("DELETE FROM ?", ("latest_update_" + self.table_name)) self.cursor.execute( - "INSERT INTO {} VALUES (?)".format("latest_update_" + self.table_name), - (time.time(),), + "INSERT INTO ? VALUES (?)", + ( + ("latest_update_" + self.table_name), + time.time(), + ), ) for mapping in self.mapping_function(): self.cursor.execute( - "INSERT INTO {} (version, sourceId) VALUES (?, ?)".format( - self.table_name - ), - (mapping[0], mapping[1]), + "INSERT INTO ? (version, sourceId) VALUES (?, ?)", + (self.table_name, mapping[0], mapping[1]), ) - data = self.cursor.execute(f"SELECT * FROM {self.table_name}").fetchall() + data = self.cursor.execute("SELECT * FROM ?", self.table_name).fetchall() self.conn.commit() return data From ec128e0df2bc3d77a3ca9ce4a204fe798c4688cf Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 14 Jul 2021 12:55:45 -0700 Subject: [PATCH 2/4] Added validation in init, marked lines for Bandit You can't use bound parameters for things like table names that aren't technically parameters in sqlite3. Moved some basic input validation into the init function and marked lines for bandit. Because bandit doesn't deal well with multi-lines with `nosec` comments, I've added descriptive comments to some select statements. Signed-off-by: Terri Oda --- cve_bin_tool/version_signature.py | 35 ++++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/cve_bin_tool/version_signature.py b/cve_bin_tool/version_signature.py index 9b4ba432a6..d80a25034a 100644 --- a/cve_bin_tool/version_signature.py +++ b/cve_bin_tool/version_signature.py @@ -9,6 +9,10 @@ from cve_bin_tool.cvedb import DISK_LOCATION_DEFAULT +class InvalidVersionSignatureTable(ValueError): + """Raised when an invalid table name is given to version_signature""" + + class VersionSignatureDb: """Methods for version signature data stored in sqlite""" @@ -16,7 +20,11 @@ def __init__(self, table_name, mapping_function, duration) -> None: """Set location on disk data cache will reside. Also sets the table name and refresh duration """ + if not table_name.isalnum(): + # Basic validation here so we can safely ignore Bandit SQL warnings + raise InvalidVersionSignatureTable self.table_name = table_name + self.update_table_name = f"latest_update_{table_name}" self.mapping_function = mapping_function self.disk_location = DISK_LOCATION_DEFAULT self.duration = duration @@ -58,16 +66,14 @@ def get_mapping_data(self): ) self.cursor.execute( - "CREATE TABLE IF NOT EXISTS {} (datestamp DATETIME PRIMARY KEY)".format( - "latest_update_" + self.table_name - ) + f"CREATE TABLE IF NOT EXISTS {self.update_table_name} (datestamp DATETIME PRIMARY KEY)" ) update_required: bool = False datestamp = self.cursor.execute( - "SELECT * FROM {}".format("latest_update_" + self.table_name) - ).fetchone() + f"SELECT * FROM {self.update_table_name}" + ).fetchone() # update_table_name validated in __init__ if datestamp and type(datestamp) is int: # Updates if the difference between current time and the time of last update is greater than duration @@ -79,22 +85,21 @@ def get_mapping_data(self): if datestamp is None or update_required: # if update is required or database is empty, fetch and insert data into database - self.cursor.execute("DELETE FROM ?", self.table_name) - self.cursor.execute("DELETE FROM ?", ("latest_update_" + self.table_name)) + self.cursor.execute(f"DELETE FROM {self.table_name}") # nosec + self.cursor.execute(f"DELETE FROM {self.update_table_name}") # nosec self.cursor.execute( - "INSERT INTO ? VALUES (?)", - ( - ("latest_update_" + self.table_name), - time.time(), - ), + f"INSERT INTO {self.update_table_name} VALUES (?)", + (time.time(),), ) for mapping in self.mapping_function(): self.cursor.execute( - "INSERT INTO ? (version, sourceId) VALUES (?, ?)", - (self.table_name, mapping[0], mapping[1]), + f"INSERT INTO {self.table_name} (version, sourceId) VALUES (?, ?)", + (mapping[0], mapping[1]), ) - data = self.cursor.execute("SELECT * FROM ?", self.table_name).fetchall() + data = self.cursor.execute( + f"SELECT * FROM {self.table_name}" + ).fetchall() # table_name validated in __init__ self.conn.commit() return data From 2e7e076841995d5610860ad516a28ad51f936237 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 14 Jul 2021 13:12:16 -0700 Subject: [PATCH 3/4] Black fix for comment --- cve_bin_tool/version_signature.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cve_bin_tool/version_signature.py b/cve_bin_tool/version_signature.py index d80a25034a..8b9a168626 100644 --- a/cve_bin_tool/version_signature.py +++ b/cve_bin_tool/version_signature.py @@ -73,7 +73,7 @@ def get_mapping_data(self): datestamp = self.cursor.execute( f"SELECT * FROM {self.update_table_name}" - ).fetchone() # update_table_name validated in __init__ + ).fetchone() # update_table_name validated in __init__ if datestamp and type(datestamp) is int: # Updates if the difference between current time and the time of last update is greater than duration From 7c75fe578e4c67465b0736a0daa463e5b3a58718 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Thu, 15 Jul 2021 09:44:40 -0700 Subject: [PATCH 4/4] Add table name to error output Co-authored-by: John Andersen --- cve_bin_tool/version_signature.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cve_bin_tool/version_signature.py b/cve_bin_tool/version_signature.py index 8b9a168626..a30d34a4ce 100644 --- a/cve_bin_tool/version_signature.py +++ b/cve_bin_tool/version_signature.py @@ -22,7 +22,7 @@ def __init__(self, table_name, mapping_function, duration) -> None: """ if not table_name.isalnum(): # Basic validation here so we can safely ignore Bandit SQL warnings - raise InvalidVersionSignatureTable + raise InvalidVersionSignatureTable(repr(table_name)) self.table_name = table_name self.update_table_name = f"latest_update_{table_name}" self.mapping_function = mapping_function