VirusTotal / yara-python

The Python interface for YARA
http://virustotal.github.io/yara/
Apache License 2.0
648 stars 179 forks source link

Callback on include #67

Closed plusvic closed 6 years ago

plusvic commented 6 years ago

Please review my latest changes. I'm intrigued about the usage of:

PyErr_Fetch(&type, &value, &traceback); ... PyErr_Restore(type, value, traceback);

What's exactly your intention with that?

edhoedt commented 6 years ago

First of all, kudos for the effort you put in correcting the mistakes I left here and there @plusvic ! The PyErr_Fetch/PyErr_Restore is there to fix a subtle Python2/Python3 inconsistency.

Take this example:

def clbk(requested, caller, namespace):
    if requested == 'CallbackWillFail.yara':
        raise Exception('random reason')
    else:
        return 'rule validrule{condition: true}'

yara.compile(source='include "CallbackWillFail.yara" include "CallbackWillSucceed.yara" rule r{condition: true}', include_callback=clbk)

Regardless of your Python version, should the first 'include' statement fail, libyara will still carry on and try to parse the second one. The difference lies here: for some reason, in Python3, if the first call to clbk raises an exception, the python interpreter will stay in that error state when the clbk is called again and will simply keep propagating that same exception. Whereas in Python2, clbk will be executed to try to include "CallbackWillSucceed.yara".

If you don't save/restore the error state before each call to your callback function and if your callback fails at any point, Python3 will produce a misleading exception stating that the last include failed (even the if last file is perfectly valid and is not the one triggering the exception).

The cleaner solution would be to make libyara stop parsing on the first include error. There is no point in parsing the rest of the file anyway since the compilation will fail in the end.

Not sure if it is clear or not. If not, please let me know, I will try to rephrase.