Skip to content

Improve exception handling#22

Merged
vojtechjelinek merged 41 commits intoNUKIB:mainfrom
jozef-sabo:fix-exception-handling
Apr 24, 2025
Merged

Improve exception handling#22
vojtechjelinek merged 41 commits intoNUKIB:mainfrom
jozef-sabo:fix-exception-handling

Conversation

@jozef-sabo
Copy link
Contributor

@jozef-sabo jozef-sabo commented Mar 31, 2025

Closes #21

Operating system was not able to equal causing NotImplementedError
Introducing a new argument `--log-level` or `-t` (for traceback) which sets the verbosity level of logging
The error handling process tries to get rid of maldump failing to execute. This is caused by throwing exceptions in Python on incorrect operations. Now, logging every step and caught exception alongside with continuing in the program execution
This step is done to make opening the files and parsing the kaitai structs and datetime objects easier to maintain and easier to log.
Until now, it used function parse_from_log(), however, Avira had no log, only filesystem entries. Now it uses parse_from_fs()
@jozef-sabo jozef-sabo force-pushed the fix-exception-handling branch from ec8cb21 to cc5d4a6 Compare April 13, 2025 23:13
@jozef-sabo
Copy link
Contributor Author

Exception handling improved, now maldump tries to do as most as possible to extract files and find metadata.
Also, for logging, use --log-level <LEVEL> switch for changing the level. Allowed levels are: DEBUG, INFO, WARNING, ERROR, CRITICAL.

Copy link
Collaborator

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Took a pass.

Comment on lines +20 to +37
def log_fn(func):
def wrapper(*args, **kwargs):
logging.debug(
"Calling function: %s, arguments: %s, keyword arguments: %s",
func.__name__,
tuple(
(
arg
if type(arg) not in {bytes, AvastParser, Element}
else "<" + type(arg).__name__ + ">"
)
for arg in args
),
kwargs,
)
return func(*args, **kwargs)

return wrapper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nešlo by to hodit o utils a neduplikovat to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presunul som to do utils. Nerobilo sa to tak preto, lebo jednotlivé loggers neboli rzlíšiteľné a nebolo jasné, z ktorého súboru sa ten log volal. Už je to fixed

Comment on lines +31 to +32
if typing.TYPE_CHECKING:
from datetime import datetime
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already on lines below.

maldump/utils.py Outdated
Comment on lines +138 to +139
if filetype:
filetype += " "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do/mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is here for the string formatting inside the logging process. You are able to provide there a type of the file, which will be printed in the format messgage. If the type is not present, it won't be present neither in the log. However, if you add pass a type, there needs to be a space, because of word spacing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I would rather not log this if it requires this. Or maybe do it in the debug strings by using f-strings? (not sure if there is an easy way to do this)

maldump/utils.py Outdated
"Cannot convert timestamp to datetime, using default",
exc_info=e,
)
timestamp = datetime.now()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want the default to be now, can we rather do start of epoch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@jozef-sabo jozef-sabo mentioned this pull request Apr 22, 2025
Copy link
Collaborator

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Took another pass.

maldump/utils.py Outdated
Comment on lines +42 to +57
tuple(
(
arg
if type(arg)
not in {
bytes,
maldump.parsers.eset_parser.EsetParser,
maldump.parsers.avast_parser.AvastParser,
maldump.parsers.avg_parser.AVGParser,
maldump.parsers.forticlient_parser.ForticlientParser,
maldump.parsers.kaspersky_parser.KasperskyParser,
maldump.parsers.malwarebytes_parser.MalwarebytesParser,
maldump.parsers.mcafee_parser.McafeeParser,
maldump.parsers.windef_parser.WindowsDefenderParser,
maldump.parsers.kaitai.forticlient_parser.ForticlientParser.Timestamp,
Element,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Also can we move the set of classes to a constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the logger function, which prints all the arguments passed to the function. When this happens, two cases may occur; either we are printting something, which can be confidential and/or long (contents of the file) or it is a class without __str__ representation. Fixed, moved.

maldump/utils.py Outdated
Comment on lines +138 to +139
if filetype:
filetype += " "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I would rather not log this if it requires this. Or maybe do it in the debug strings by using f-strings? (not sure if there is an easy way to do this)

Copy link
Collaborator

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@vojtechjelinek vojtechjelinek merged commit 2a46c82 into NUKIB:main Apr 24, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve exception handling

2 participants