VirusTotal / yara

The pattern matching swiss knife
https://virustotal.github.io/yara/
BSD 3-Clause "New" or "Revised" License
8.32k stars 1.45k forks source link

Alignment in 32-bits. Again #2070

Open detrio333 opened 6 months ago

detrio333 commented 6 months ago

I faced problem which solved in https://github.com/VirusTotal/yara/commit/9c26038171edcdc084370562703f22e649067125 Yara rules compiled by yara64.exe don't work with yara32.exe. I saw the problem was solved by addition field 'uint32_t unused;', but I think it is non-persistent soltuion. Adding one more uint32_t raise the probem again

To Reproduce 1) simple rule. file rules.yara

import "string"

rule StringLength
{
    condition:
        string.length("123") == 3
}

2) yarac64.exe "D:\rules.yara" d:\rules.yarac 3) 64 bit version run ok

yara64.exe -C "D:\rules.yarac" "D:\1.txt"
StringLength D:\1.txt

4) 32 bit version don't work

yara32.exe -C "D:\rules.yarac" "D:\1.txt"

I obtain in debug mode rule_table is collapsed image 5) Ubuntu compiled version run as expected for 32 and 64 bit both

Please complete the following information:

Additional context I can see problem with aligment in macro

#define DECLARE_REFERENCE(type, name) \
  union                               \
  {                                   \
    type name;                        \
    YR_ARENA_REF name##_;             \
  } YR_ALIGN(8)

when

#if defined(__GNUC__)
#define YR_ALIGN(n) __attribute__((aligned(n)))
#elif defined(_MSC_VER)
#define YR_ALIGN(n) __declspec(align(n))
#else
#define YR_ALIGN(n)
#endif

According microsoft documentaion https://learn.microsoft.com/en-us/cpp/cpp/align-cpp?view=msvc-170 aling command must go before struct or union declarion. For gcc __attribute__((aligned(n))) can be wriiten before and after both.

Using YR_ALIGN everywhere except DECLARE_REFERENCE is before struct, union. For example

struct YR_EXTERNAL_VARIABLE
{
  int32_t type;

  YR_ALIGN(8) union
  {
    int64_t i;
    double f;
    char* s;
  } value;

  DECLARE_REFERENCE(const char*, identifier);
};

It work nice. I think there were no problems before because YR_RULE contained 2 int32_t field. Adding uint32_t required_strings caues misalining in rule loading. 'uint32_t unused;' is non-persistent soltuion, because another new field also break the loading. I suggest to move YR_ALIGN(8) to try avoid same problem in future

#define DECLARE_REFERENCE(type, name) \
  YR_ALIGN(8) union                   \
  {                                   \
    type name;                        \
    YR_ARENA_REF name##_;             \
  }