VirusTotal / yara-python

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

Support xor_value in returned strings. #210

Closed wxsBSD closed 1 year ago

wxsBSD commented 2 years ago

Extend the tuple that represents an instance of a match to include the xor key. This breaks all existing scripts that are unpacking the tuple, which I'm not very happy with.

This also updates the submodule to use the latest master so that I can get the new xor key values.

Also, adds a fix to get yara building here by defining BUCKETS_128 and CHECKSUM_1B as needed by the new tlsh stuff (discussed with @metthal).

wxsBSD commented 2 years ago

I'm not super happy with just extending the tuple here as it will break existing scripts that are unpacking the tuple in assignment. They will have to go from (offset, identifier, data) = ... to (offset, identifier, data, xor) = .... The only scripts that won't break are those that do tup = ...; tup[2] which I don't think would be very common.

Since this is going to break a lot of scripts, I wonder if it makes sense to completely remove the tuple entirely and replace it with an actual object with members instead. Doing so would make it more extensible in the future. I could even support a plaintext method that will take the matched data, apply the xor key and return the plaintext string automatically.

Assuming this PR (or some variant of it) is a good idea I'll update the docs with whatever is decided after it is merged.

wxsBSD commented 2 years ago

I'm going to update this to use an actual object so it is more extensible in the future.

wxsBSD commented 2 years ago

The commit I just made gives more detail on the changes. I'd love to hear more about what I should do with richcompare for the new objects. It is unclear to me how I want to compare two strings, or two string instances, so I left them out for now. I can revisit that after some more discussion I think.

wxsBSD commented 2 years ago

One more thing, I noticed the tests were always using self.assertTrue() when it is much nicer to use the other assertions. I switched my tests in this PR to use self.assertEqual() (or other things if needed) and will go through all the other tests in a future PR if desired. It makes it much easier to debug failed tests when it tells you the failed values (instead of just "False is not true" messages =b).

wxsBSD commented 2 years ago

I'll update the docs in the main yara repo once this is merged.

wxsBSD commented 1 year ago

Closing out as it has been merged into my "next" branch for inclusion.