Libguestfs - Oct 2021 - [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes

On Tue, Oct 12, 2021 at 09:16:10AM -0500, Eric Blake
wrote:
> On Tue, Oct 12, 2021 at 12:36:27AM +0200, Laszlo Ersek wrote:
> > The prototype of yara_rules_callback() is:
> > 
> > > static int
> > > yara_rules_callback (int code, void *message, void *data)
> > 
> > however, in Yara commit 2b121b166d25 ("Track string matches using
> > YR_SCAN_CONTEXT.", 2020-02-27), which was included in the
upstream v4.0.0
> > release, the rules callback prototype was changed as follows:
> > 
> > > diff --git a/libyara/include/yara/types.h
b/libyara/include/yara/types.h
> > > index cad095cd70c2..f415033c4aa6 100644
> > > --- a/libyara/include/yara/types.h
> > > +++ b/libyara/include/yara/types.h
> > > @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
> > >
> > >
> > >  typedef int (*YR_CALLBACK_FUNC)(
> > > +    YR_SCAN_CONTEXT* context,
> > >      int message,
> > >      void* message_data,
> > >      void* user_data);
> 
> Do we intend to compile against both older and newer versions of Yara,
> in which case we'd need a configure-time probe of which variant we
> must compile against?  I could not quickly find documentation of a
> minimum version of Yara that we are willing to support, at least not
> in README or HACKING.
FWIW as Yara is a very niche feature for the digital forensics people
I'm fine with supporting only the latest or only the most convenient
version.  Good idea to document which version we support though.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On 10/12/21 17:27, Richard W.M. Jones wrote:
> On Tue, Oct 12, 2021 at 09:16:10AM -0500, Eric Blake wrote:
>> On Tue, Oct 12, 2021 at 12:36:27AM +0200, Laszlo Ersek wrote:
>>> The prototype of yara_rules_callback() is:
>>>
>>>> static int
>>>> yara_rules_callback (int code, void *message, void *data)
>>>
>>> however, in Yara commit 2b121b166d25 ("Track string matches
using
>>> YR_SCAN_CONTEXT.", 2020-02-27), which was included in the
upstream v4.0.0
>>> release, the rules callback prototype was changed as follows:
>>>
>>>> diff --git a/libyara/include/yara/types.h
b/libyara/include/yara/types.h
>>>> index cad095cd70c2..f415033c4aa6 100644
>>>> --- a/libyara/include/yara/types.h
>>>> +++ b/libyara/include/yara/types.h
>>>> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>>>>
>>>>
>>>>  typedef int (*YR_CALLBACK_FUNC)(
>>>> +    YR_SCAN_CONTEXT* context,
>>>>      int message,
>>>>      void* message_data,
>>>>      void* user_data);
>>
>> Do we intend to compile against both older and newer versions of Yara,
>> in which case we'd need a configure-time probe of which variant we
>> must compile against?  I could not quickly find documentation of a
>> minimum version of Yara that we are willing to support, at least not
>> in README or HACKING.
> 
> FWIW as Yara is a very niche feature for the digital forensics people
> I'm fine with supporting only the latest or only the most convenient
> version.  Good idea to document which version we support though.
I avoided spelling out a Yara version in either documentation or in
build configuration -- even if we restrict our dependency now to >= 4.0,
if the Yara authors break the public APIs again, e.g. in 5.0 or
whatever, our dependency check will be useless, and we'll only be able
to rely on the compilation failure again. I thought we could just put
pre-4.0 Yara versions to the sword with the exact same effort --
compilation failure.

Now, requiring precisely Yara 4.0 or 4.1 would be a different matter,
but that would turn into an unnecessary annoyance once upstream Yara
advanced *without* breaking the APIs...

I can follow up with a patch for the Yara dependency description in the
"docs/guestfs-building.pod" file (plus a dozen *.po files I guess? not
sure), but I'm unsure what to say. "We currently support >= 4.0
versions, unless upstream has broken their public API since 4.0"?

(... And so we've arrived at interface introspection here too. :))

Thanks
Laszlo

On 10/12/21 17:27, Richard W.M. Jones wrote:
> On Tue, Oct 12, 2021 at 09:16:10AM -0500, Eric Blake wrote:
>> On Tue, Oct 12, 2021 at 12:36:27AM +0200, Laszlo Ersek wrote:
>>> The prototype of yara_rules_callback() is:
>>>
>>>> static int
>>>> yara_rules_callback (int code, void *message, void *data)
>>>
>>> however, in Yara commit 2b121b166d25 ("Track string matches
using
>>> YR_SCAN_CONTEXT.", 2020-02-27), which was included in the
upstream v4.0.0
>>> release, the rules callback prototype was changed as follows:
>>>
>>>> diff --git a/libyara/include/yara/types.h
b/libyara/include/yara/types.h
>>>> index cad095cd70c2..f415033c4aa6 100644
>>>> --- a/libyara/include/yara/types.h
>>>> +++ b/libyara/include/yara/types.h
>>>> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>>>>
>>>>
>>>>  typedef int (*YR_CALLBACK_FUNC)(
>>>> +    YR_SCAN_CONTEXT* context,
>>>>      int message,
>>>>      void* message_data,
>>>>      void* user_data);
>>
>> Do we intend to compile against both older and newer versions of Yara,
>> in which case we'd need a configure-time probe of which variant we
>> must compile against?  I could not quickly find documentation of a
>> minimum version of Yara that we are willing to support, at least not
>> in README or HACKING.
> 
> FWIW as Yara is a very niche feature for the digital forensics people
> I'm fine with supporting only the latest or only the most convenient
> version.  Good idea to document which version we support though.
I'd also like to clarify that I didn't ignore (part of) the feedback I
received between my posting the series and my merging it. Instead, I
simply didn't see it. I process emails in infrequent batches, in order
to improve my throughput (keep context switches as infrequent as
possible). Thus, I've not checked email between (approx.) Rich's
"ACK
series" and now.

Thanks,
Laszlo