VirusTotal / yara

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

calloc behavior on AIX platform #1980

Closed asalih closed 10 months ago

asalih commented 11 months ago

According to the documentation AIX returns NULL when requested size is 0 but also returns NULL if you provide 0 to count parameter. In this example rule below, there is no strings definition in it, rule successfully compiles with given variable but afterwards when I try to create a scanner with the compiled rules, yr_scanner_create function returns insufficient memory error. The reason is; new_scanner->matches is NULL because of the calloc behavior. To avoid this, #define _LINUX_SOURCE_COMPAT must be added to mem.c.

rule find_by_name 
{
    meta:
        description = "Find files by name."

    condition:
        file_name == "Task.dat"
}
hillu commented 11 months ago

Interesting. The manpage for malloc and friends on my Debian system list this as nonportable behavior. IMHO, the correct response is to rewrite those allocations rather than telling the compiler to use glibc-compatible variants of those library functions.

asalih commented 10 months ago

Thank you for the fix, @hillu. The only concern with these changes is that, from now on, in addition to the NULL check on a calloc call, there must also be relevant count control, similar to your additional checks. However, due to the common behavior of calloc on Linux, it might be overlooked and become implicit knowledge, potentially leading to oversights. Adding the _LINUX_SOURCE_COMPAT macro with a conditional compilation directive can prevent future problems, what do you think?

plusvic commented 10 months ago

I have similar concerns. It's very easy to re-introduce issues like this in the future.

hillu commented 10 months ago

Since we have yr_{malloc,calloc,free} wrappers, we could put some preprocessor magic there that implements the GNU libc compatible behavior if needed. I'm sure that we are not the first authors to stumble into this, therefore I'm pretty confident that there are existing checks for Autoonf…