CERT-Polska / karton-archive-extractor

Extractor of various archive formats for Karton framework
https://github.com/CERT-Polska/karton
GNU General Public License v3.0
5 stars 6 forks source link

Initialise archive file name in all code paths #36

Closed michaelweiser closed 2 years ago

michaelweiser commented 2 years ago

Variable fname will stay uninitialised if the sample has no name. This will lead to an exception further down when the local file name is constructed for extraction:

[ERROR] karton.archive-extractor: Failed to process task - 4900d4bd-13d7-406e-b7f3-e2f195b63769
Traceback (most recent call last):
  File "/opt/karton-archive-extractor/lib/python3.8/site-packages/karton/core/karton.py", line 178, in internal_process
self.process(self.current_task)
  File "/opt/karton-archive-extractor/lib/python3.8/site-packages/karton/archive_extractor/archive_extractor.py", line 58, in process
    filepath = f"{dir_name}/{fname}"
UnboundLocalError: local variable 'fname' referenced before assignment

This can be avoided by initialising the variable name beforehand so it is always initialised and only overridden in the case that a sample name is at hand.

Related question: Should maybe that default archive name also be supplemented with the classifier extension if available? Something like so:

diff --git a/karton/archive_extractor/archive_extractor.py b/karton/archive_extractor/archive_extractor.py
index 190457a..e460937 100644
--- a/karton/archive_extractor/archive_extractor.py
+++ b/karton/archive_extractor/archive_extractor.py
@@ -40,9 +40,9 @@ class ArchiveExtractor(Karton):
             if sample.name:
                 fname = sample.name

-                classifier_extension = "." + task.headers["extension"]
-                if classifier_extension and not fname.endswith(classifier_extension):
-                    fname += classifier_extension
+            classifier_extension = "." + task.headers["extension"]
+            if classifier_extension and not fname.endswith(classifier_extension):
+                fname += classifier_extension
         except Exception as e:
             self.log.warning("Exception during extraction: %r", e)
stephanhendl commented 2 years ago

I can confirm that with the patch applied the errors were gone.

michaelweiser commented 2 years ago

Sorry for taking so long to respond. I've added the proposed unconditional extension supplement for the default archive name as well in a separate commit.