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

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.
> Of course, evaluating this expression would be undefined behavior, but in
> the GCC extension expression
> 
>   typeof (*(YR_CALLBACK_FUNC)NULL)
> 
> the operand of the "typeof" operator is never evaluated, as it
does not
> have a variably modified type. We can therefore use this "typeof"
in the
> same role as HELLO_FUNC had in the above example.
Clever.
> +/* Typedefs that effectively strip the pointer derivation from Yara's
> + * YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC types, using GCC's
"typeof"
> + * extension.
> + */
> +typedef typeof (*(YR_CALLBACK_FUNC)NULL)          guestfs_yr_callback;
> +typedef typeof (*(YR_COMPILER_CALLBACK_FUNC)NULL)
guestfs_yr_compiler_callback;
> +
> +/* Declarations of our callback functions expressed in terms of Yara's
> + * typedefs. Note: these are *function declarations*.
> + */
> +static guestfs_yr_callback          yara_rules_callback;
> +static guestfs_yr_compiler_callback compile_error_callback;
> +
As written, this portion of the code will compile with either old or
new Yara...
>  /* Has one FileIn parameter.
>   * Takes optional arguments, consult optargs_bitmask.
>   */
> @@ -210,7 +221,7 @@ compile_rules_file (const char *rules_path)
>   */
>  static void
>  compile_error_callback (int level, const char *name, int line,
> -                        const char *message, void *data)
> +                        const YR_RULE* rule, const char *message, void
*data)
...but this will cause compile failure on old Yara (redefinition to an
incompatible type); if we want to support both versions, we need
#ifdef alternatives depending on the version of Yara we are building
against.

[I did not check whether the Yara project properly bumped their .so
version to match their incompatible ABI change; hopefully they did, so
that anyone using dynamic linking against older libraries will be
forced to recompile for the new .so, rather than being hit with
runtime failures as you encountered in the libguestfs testsuite.  But
if not, that's a bug report for a different mailing list]

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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