VirusTotal / yara

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

Valgrind errors #921

Closed teoring closed 5 years ago

teoring commented 6 years ago

Next snippet of the code cause valgrind to report mentioned below errors:

    void YaraRulesProvider::compileRule( const std::string& aSourceRuleFilePath )
    {
        // Once yr_compiler_get_rules() is invoked you can not add more sources to the compiler
        // that means for each compilation compiler has to be reinitialized. Also setups a callback
        // for the compilation errors so the errors will be saved in \ref mCompilationErrors
        initializeCompiler();

        std::unique_ptr<FILE, void (*)(FILE *)> sourceFile( fopen( aSourceRuleFilePath.c_str(), "r" ), fileDeleter );
        if ( ! sourceFile )
        {
            sl::Log::warning() << "[YaraDetector] Couldn't load source rule.\n\t[Path]:"
                << aSourceRuleFilePath << "\n\t[Error]:" << std::string( std::strerror( errno ) ) << "\n";
            return;
        }

        if ( yr_compiler_add_file( mCompiler.get(), sourceFile.get() , nullptr, nullptr ) != 0 )
        {
            sl::Log::warning() << "[YaraDetector] Unable to compile the YARA rule.\n\t[FilePath]:"
                << aSourceRuleFilePath << "\n";

            // After unsuccessful compilation YARA rule is removed from the filesystem
            boost::system::error_code err;
            boost::filesystem::remove( boost::filesystem::path( aSourceRuleFilePath ), err );
            return;
        }

        YR_RULES* rules = nullptr;
        if ( yr_compiler_get_rules( mCompiler.get(), &rules ) == ERROR_SUCCESS )
        {
            mRules.emplace_back( rules, rulesDeleter );

            boost::filesystem::path nameExtractor( aSourceRuleFilePath );
            std::string ruleName = mCompiledRulesPath + "/" + nameExtractor.stem().string() + ".out";

            boost::system::error_code err;
            boost::filesystem::remove( boost::filesystem::path( ruleName ), err );

            int retcode = yr_rules_save( rules, ruleName.c_str() );
            if( retcode != ERROR_SUCCESS )
            {
                sl::Log::error() << "[YaraDetector] Couldn't save compiled YARA rules to the filesystem.\n\t[Path]:"
                    << mCompiledRulesPath << "\n\t[Retcode]: " << std::to_string( retcode ) << "\n";
            }
        } else
        {
            sl::Log::error() << "[YaraDetector] Not enought memory to get the YARA rule.\n";
        }
    }
==20232== Syscall param write(buf) points to uninitialised byte(s)
==20232==    at 0x1B1872DD: ??? (syscall-template.S:84)
==20232==    by 0x1B108BFE: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1263)
==20232==    by 0x1B109389: new_do_write (fileops.c:518)
==20232==    by 0x1B109389: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1342)
==20232==    by 0x1B0FE7BA: fwrite (iofwrite.c:39)
==20232==    by 0x1A5B7F6E: yr_arena_save_stream (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1A5D9EAF: yr_rules_save (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1784AA64: commons::yara::YaraRulesProvider::compileRule(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (YaraBroker.cc:431)
==20232==    by 0x1784B950: commons::yara::YaraRulesProvider::compileRules() (YaraBroker.cc:467)
==20232==    by 0x1784C03C: commons::yara::YaraRulesProvider::initialize() (YaraBroker.cc:489)
==20232==    by 0x1784C637: commons::yara::YaraBroker::initialize() (YaraBroker.cc:520)
==20232==    by 0x1242C08: void std::_Mem_fn_base<void (commons::yara::YaraBroker::*)(), true>::operator()<, void>(commons::yara::YaraBroker*) const (in /home/artem.shymanskyi/Development/nids_yara/tests/build/Debug/unit_tests)
==20232==    by 0x1241957: void std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (functional:1531)
==20232==  Address 0x20a963a8 is 7,080 bytes inside a block of size 23,640 alloc'd
==20232==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20232==    by 0x1A5B7A89: yr_arena_duplicate (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1A5BB91D: yr_compiler_get_rules (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1784A7B6: commons::yara::YaraRulesProvider::compileRule(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (YaraBroker.cc:421)
==20232==    by 0x1784B950: commons::yara::YaraRulesProvider::compileRules() (YaraBroker.cc:467)
==20232==    by 0x1784C03C: commons::yara::YaraRulesProvider::initialize() (YaraBroker.cc:489)
==20232==    by 0x1784C637: commons::yara::YaraBroker::initialize() (YaraBroker.cc:520)
==20232==    by 0x1242C08: void std::_Mem_fn_base<void (commons::yara::YaraBroker::*)(), true>::operator()<, void>(commons::yara::YaraBroker*) const (in /home/artem.shymanskyi/Development/nids_yara/tests/build/Debug/unit_tests)
==20232==    by 0x1241957: void std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (functional:1531)
==20232==    by 0x12407E3: std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)>::operator()() (functional:1520)
==20232==    by 0x123F40D: void std::__once_call_impl<std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)> >() (mutex:706)
==20232==    by 0x15B60A98: __pthread_once_slow (pthread_once.c:116)
==20232== 
{
   <insert_a_suppression_name_here>
   Memcheck:Param
   write(buf)
   obj:/lib/x86_64-linux-gnu/libc-2.23.so
   fun:_IO_file_write@@GLIBC_2.2.5
   fun:new_do_write
   fun:_IO_file_xsputn@@GLIBC_2.2.5
   fun:fwrite
   fun:yr_arena_save_stream
   fun:yr_rules_save
   fun:_ZN7commons4yara17YaraRulesProvider11compileRuleERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
   fun:_ZN7commons4yara17YaraRulesProvider12compileRulesEv
   fun:_ZN7commons4yara17YaraRulesProvider10initializeEv
   fun:_ZN7commons4yara10YaraBroker10initializeEv
   fun:_ZNKSt12_Mem_fn_baseIMN7commons4yara10YaraBrokerEFvvELb1EEclIIEvEEvPS2_DpOT_
   fun:_ZNSt12_Bind_simpleIFSt7_Mem_fnIMN7commons4yara10YaraBrokerEFvvEEPS3_EE9_M_invokeIILm0EEEEvSt12_Index_tupleIIXspT_EEE
}

==20232== Use of uninitialised value of size 8
==20232==    at 0x1A5C1F48: yr_hash (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1A5B7F96: yr_arena_save_stream (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1A5D9EAF: yr_rules_save (in /usr/local/lib/libyara.so.3.7.1)
==20232==    by 0x1784AA64: commons::yara::YaraRulesProvider::compileRule(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (YaraBroker.cc:431)
==20232==    by 0x1784B950: commons::yara::YaraRulesProvider::compileRules() (YaraBroker.cc:467)
==20232==    by 0x1784C03C: commons::yara::YaraRulesProvider::initialize() (YaraBroker.cc:489)
==20232==    by 0x1784C637: commons::yara::YaraBroker::initialize() (YaraBroker.cc:520)
==20232==    by 0x1242C08: void std::_Mem_fn_base<void (commons::yara::YaraBroker::*)(), true>::operator()<, void>(commons::yara::YaraBroker*) const (in /home/artem.shymanskyi/Development/nids_yara/tests/build/Debug/unit_tests)
==20232==    by 0x1241957: void std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (functional:1531)
==20232==    by 0x12407E3: std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)>::operator()() (functional:1520)
==20232==    by 0x123F40D: void std::__once_call_impl<std::_Bind_simple<std::_Mem_fn<void (commons::yara::YaraBroker::*)()> (commons::yara::YaraBroker*)> >() (mutex:706)
==20232==    by 0x15B60A98: __pthread_once_slow (pthread_once.c:116)
==20232== 
{
   <insert_a_suppression_name_here>
   Memcheck:Value8
   fun:yr_hash
   fun:yr_arena_save_stream
   fun:yr_rules_save
   fun:_ZN7commons4yara17YaraRulesProvider11compileRuleERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
   fun:_ZN7commons4yara17YaraRulesProvider12compileRulesEv
   fun:_ZN7commons4yara17YaraRulesProvider10initializeEv
   fun:_ZN7commons4yara10YaraBroker10initializeEv
   fun:_ZNKSt12_Mem_fn_baseIMN7commons4yara10YaraBrokerEFvvELb1EEclIIEvEEvPS2_DpOT_
   fun:_ZNSt12_Bind_simpleIFSt7_Mem_fnIMN7commons4yara10YaraBrokerEFvvEEPS3_EE9_M_invokeIILm0EEEEvSt12_Index_tupleIIXspT_EEE
   fun:_ZNSt12_Bind_simpleIFSt7_Mem_fnIMN7commons4yara10YaraBrokerEFvvEEPS3_EEclEv
   fun:_ZSt16__once_call_implISt12_Bind_simpleIFSt7_Mem_fnIMN7commons4yara10YaraBrokerEFvvEEPS4_EEEvv
   fun:__pthread_once_slow
}

Are those are expected errors?

plusvic commented 6 years ago

My guess is that YARA is that calls to yr_stream_write are failing in:

https://github.com/VirusTotal/yara/blob/master/libyara/arena.c#L1094

I should check that the number of returned bytes are the expected ones. Can you make sure that those calls to yr_stream_write are not returning 0?

plusvic commented 6 years ago

Please apply these changes and let me know if the problem persists: https://github.com/VirusTotal/yara/commit/fc2e94e51636814f469420b9a4557adc5493f7d6

teoring commented 6 years ago

Sorry for the late response, I will apply the patch and will provide the result as soon as will find the time.

teoring commented 5 years ago

Issue can be closed. I confirm valgrind do not report any issue with the mentioned patch.