ECHO macro causes CLI tool to crash at runtime (Windows)

[1ndahous3]

The bug was discovered when the CLI tool crashed while loading a simple invalid rule:

rule test{
    strings:
        $s = /\
}

Actually MSVC CRT parameter validation failed at runtime:

>   yara64.exe!_write_nolock(int fh, const void * buffer, unsigned int buffer_size, __crt_cached_ptd_host & ptd)
    yara64.exe!_write_internal(int fh, const void * buffer, unsigned int size, __crt_cached_ptd_host & ptd)
    yara64.exe!__acrt_stdio_flush_nolock(_iobuf * public_stream, __crt_cached_ptd_host & ptd)
    yara64.exe!__acrt_stdio_end_temporary_buffering_nolock(bool flag, _iobuf * public_stream, __crt_cached_ptd_host & ptd)
    yara64.exe!__acrt_stdio_temporary_buffering_guard::~__acrt_stdio_temporary_buffering_guard()
    yara64.exe!_fwrite_internal::__l2::<lambda>()
    yara64.exe!__crt_seh_guarded_call<unsigned __int64>::operator()<void <lambda>(void),unsigned __int64 <lambda>(void) &,void <lambda>(void)>(__acrt_lock_stream_and_call::__l2::void <lambda>(void) && setup, _fwrite_internal::__l2::unsigned __int64 <lambda>(void) & action, __acrt_lock_stream_and_call::__l2::void <lambda>(void) && cleanup)
    yara64.exe!__acrt_lock_stream_and_call<unsigned __int64 <lambda>(void)>(_iobuf * const stream, _fwrite_internal::__l2::unsigned __int64 <lambda>(void) && action)
    yara64.exe!_fwrite_internal(const void * const buffer, const unsigned __int64 size, const unsigned __int64 count, _iobuf * const stream, __crt_cached_ptd_host & ptd)
    yara64.exe!fwrite(const void * buffer, unsigned __int64 size, unsigned __int64 count, _iobuf * stream)
    yara64.exe!yara_yylex(YYSTYPE * yylval_param, void * yyscanner, _YR_COMPILER * compiler)
    yara64.exe!yara_yyparse(void * yyscanner, _YR_COMPILER * compiler)
    yara64.exe!yr_lex_parse_rules_file(_iobuf * rules_file, _YR_COMPILER * compiler)
    yara64.exe!yr_compiler_add_file(_YR_COMPILER * compiler, _iobuf * rules_file, const char * namespace_, const char * file_name)
    yara64.exe!compile_files(_YR_COMPILER * compiler, int argc, const wchar_t * * argv)
    yara64.exe!wmain(int argc, const wchar_t * * argv)

I then figured out that the ECHO macro called with the \ symbol tries to output 1 character directly to the console. https://github.com/VirusTotal/yara/blob/01032a6cb26313e1838669418186563a4e5cfc2d/libyara/lexer.c#L1183-L1189

Failed CRT runtime check:

    // If the file is open for Unicode, the buffer size must always be even:
    if (fh_textmode == __crt_lowio_text_mode::utf16le || fh_textmode == __crt_lowio_text_mode::utf8)
    {
        _UCRT_VALIDATE_CLEAR_OSSERR_RETURN(ptd, buffer_size % 2 == 0, EINVAL, -1);
    }

So, if we read the fwrite documentation, we see a note:

If either stream or buffer is a null pointer, or if an odd number of bytes to be written is specified in Unicode mode, the function invokes the invalid parameter handler, as described in Parameter validation.

And the CLI tool configures the stdout to write UTF8 data: https://github.com/VirusTotal/yara/blob/01032a6cb26313e1838669418186563a4e5cfc2d/cli/yara.c#L1472-L1478

Since fwrite depends on the encoding, I think writing random bytes from a buffer (rules) to stdout (or any FILE *) without context (right at the beginning) is a bad idea. Maybe it can just be removed?

[1ndahous3]

@plusvic is it ok to just remove the ECHO macro?

[plusvic]

The lexer.c file is automatically generated by lex, so we can't simply remove it. But we can define ECHO ourselves as an empty string in lexer.h.

[1ndahous3]

@plusvic IMHO version with redefined macro is a little better in terms of output info.

With redefined macro:

error: rule "test_crash" in \test.yar(4): unterminated regular expression error: rule "test_crash" in \test.yar(4): syntax error, unexpected end of file, expecting text string

With lexer the "nodefault" option:

error: rule "test_crash" in \test.yar(3): flex scanner jammed

[plusvic]

Absolutely, I think I'll adopt your solution then.