Provide duplicate meta keys as list instead of just the last seen value by metthal · Pull Request #74 · VirusTotal/yara-python

metthal · 2018-01-23T10:31:52Z

When I run regular yara tool with -m flag to print meta information I can get all meta keys even if there are duplicates. Take this YARA file:

rule dummy {
	meta:
		key = "a"
		key = "b"
		key = "c"
		another_key = "d"
	condition:
		true
}

Here is an example of how I can obtain all of them:

$ yara -m /tmp/test.yar /tmp/test.yar 
dummy [key="a",key="b",key="c",another_key="d"] /tmp/test.yar

However, there is now way how to obtain all of them through Python API. I only get the value of the key which is mentioned as last in the file because ordinary dict is used. My change alters this behavior and allows you to obtain all the values. Simply, if there are any duplicates, it starts putting the values in the list.

Before my change:

$ python
>>> import yara
>>> r = yara.compile('/tmp/test.yar')
>>> m = r.match(data='abc')
>>> print(m[0].meta)
{'key': 'c', 'another_key': 'd'}

After my change:

$ python
>>> import yara
>>> r = yara.compile('/tmp/test.yar')
>>> m = r.match(data='abc')
>>> print(m[0].meta)
{'key': ['a', 'b', 'c'], 'another_key': 'd'}

Note: I am not that familiar with Python C API so I am not sure whether I put Py_DECREF wherever it's necessary.

wxsBSD · 2018-01-23T13:06:06Z

This is going to break some of our existing code at my employer. I can only guess we are not the only ones with this problem too.

metthal · 2018-01-23T13:16:42Z

What would you suggest then? Make this behavior optional with some flag? We rely on this behavior. YARA language documentation does not specify whether meta keys need to be unique (at least I didn't find it) so we assumed that they don't have to be since YARA tool provides you with all the meta information, even duplicates.

wxsBSD · 2018-01-23T13:20:45Z

I didn’t realize the YARA tool provides duplicates. I’m probably still against this change, and just documenting it. This is really a python thing...

metthal · 2018-01-23T13:37:37Z

Do your rulesets contain meta with duplicate keys? Because if you weren't even aware of it and weren't using duplicates, my change does not break anything in that case. You can notice that in the examples I provided. another_key is still just a string.

wxsBSD · 2018-01-23T13:47:15Z

I have some code which assumes values are not a list. I can easily fix this, but others may be unaware of this change at first.

plusvic · 2018-01-26T16:54:53Z

Instead of introducing a backward-incompatible API change this could be implemented as a new function for the Match object. The function could be named get_meta and it could receive a boolean argument allow_duplicates, that is false by default. So, match.meta would be equivalent to match.get_meta(), but match.get_meta(allow_duplicates=True) could return a list in case of duplications.

This way existing programs are not affected, but you have a way of getting all meta values with the same name. Thought?

metthal · 2018-01-30T11:44:13Z

I like that idea. I'll implement it, I just need to get through all the stuff I am bogged down with right now.

metthal · 2018-01-31T15:38:20Z

So I added get_meta() as proposed by @plusvic. This is how the input file looks like:

rule a {
	meta:
		key = "a"
		key = "b"
		key = "c"
	condition:
		true
}

This is code that uses get_meta():

import yara

r = yara.compile('/tmp/test.yar')
m = r.match(data='abc')
print(m[0].meta)
print(m[0].get_meta())
print(m[0].get_meta(allow_duplicates=True))

And the output:

{'key': 'c'}
{'key': 'c'}
{'key': ['a', 'b', 'c']}

metas with no or all duplicates

metthal · 2018-11-15T14:18:14Z

Bumping the PR since it somehow got completely forgotten.

Northern-Lights · 2020-02-25T00:44:43Z

Bump. It would be great to have the bindings mirror the libyara API.

plusvic · 2021-08-12T14:45:06Z

yara-python.c

    READONLY,
    "Dictionary with offsets and strings that matched the file"
  },
+  {


There's no need for exporting this field to Python programs, right? If we already have the get_meta() function, we better prevent people from doing match._meta_with_dups.

plusvic · 2021-08-12T14:58:36Z

yara-python.c

+      }
+    }
+    else
+      PyDict_SetItemString(meta_list_with_dups, meta->identifier, object);


I think that values in meta_list_with_dups should be always a list, even if the key has a single value. These way you don't need to take into account whether the value is a list or a single value.

In my previous description I said:

match.get_meta(allow_duplicates=True) could return a list in case of duplications.

But after a second thought I think I'm not sure about returning a list only in case of duplications. That shouldn't be a big issue given Python's dynamic nature, but it may be less error prone if allow_duplicates=True means that values are always a list even if it has a single item. Thoughts?

Hey hey! I agree that this new function should always return a list, regardless of the number of items in the list. This makes the function's output type consistent so as user I don't need to add conditions to check the type of response.

I (as well as others) are interested in this PR, so I'd be happy to pick up where @metthal left off!

Oh, sorry, I completely missed the fact that @plusvic reviewed my PR. I'm open to change the behavior in this PR.

Out of curiosity, why would we need to maintain a flag to allow duplicate values? I suppose there's no harm to have it but just wondering.

If you just want a straight Dict[str, str] object, you could always just use the Match.meta attribute, which is legacy for a lot of potential users.

BUT if you want all the metadata regardless of duplication, a la Match.get_meta() (or Match.get_all_meta() to be concise 😉), then you can always expect to get Dict[str, List[str]] back even if there's one item per list.

Mind you I'm still reading through the changes to make sure I understand them properly😀

EDIT: I also agree that it might not be necessary to expose an attribute containing the full meta especially if it's given as function output which the user will call. See: #74 (comment)

plusvic · 2022-05-20T11:47:35Z

Already implemented in #201

Tasssadar mentioned this pull request Nov 15, 2018

Meta variables with duplicate name are lost hillu/go-yara#44

Closed

metthal added 2 commits November 15, 2018 15:06

Provide duplicate meta keys as list instead of just the last seen value

ce3f1db

Added get_meta() method to Match class which is capable of returning

55031f8

metas with no or all duplicates

metthal force-pushed the duplicate-meta-keys branch from 62a5f26 to 55031f8 Compare November 15, 2018 14:10

wxsBSD mentioned this pull request Feb 19, 2020

meta in results do not mirror those in the command line tool or C API #129

Closed

malvidin mentioned this pull request Apr 22, 2020

It should not be necessary to return a list as metadata for returning python types plyara/plyara#49

Closed

plusvic reviewed Aug 12, 2021

View reviewed changes

cccs-rs mentioned this pull request Mar 9, 2022

Allow metadata to contain a list of values #201

Merged

wxsBSD mentioned this pull request Apr 22, 2022

yara-python <-> yara inconsistent handling of metadata #205

Closed

plusvic closed this May 20, 2022

Conversation

metthal commented Jan 23, 2018

Uh oh!

wxsBSD commented Jan 23, 2018

Uh oh!

metthal commented Jan 23, 2018

Uh oh!

wxsBSD commented Jan 23, 2018

Uh oh!

metthal commented Jan 23, 2018

Uh oh!

wxsBSD commented Jan 23, 2018

Uh oh!

plusvic commented Jan 26, 2018

Uh oh!

metthal commented Jan 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metthal commented Jan 31, 2018

Uh oh!

metthal commented Nov 15, 2018

Uh oh!

Northern-Lights commented Feb 25, 2020

Uh oh!

plusvic Aug 12, 2021

Choose a reason for hiding this comment

Uh oh!

plusvic Aug 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cccs-rs Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

metthal Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

cccs-rs Mar 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plusvic commented May 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

metthal commented Jan 30, 2018 •

edited

Loading

plusvic Aug 12, 2021 •

edited

Loading

cccs-rs Mar 2, 2022 •

edited

Loading