Cisco-Talos / clamav

ClamAV - Documentation is here: https://docs.clamav.net
https://www.clamav.net/
GNU General Public License v2.0
4.23k stars 687 forks source link

valgrind tests fail with 0.105.0 (Alpine) #578

Open SecT0uch opened 2 years ago

SecT0uch commented 2 years ago

Describe the bug

When using the 0.105.0 tar.gz on Alpine (latest), I installed all the dependencies (including the new ones). Then, ctest returns (see attached verbose logs):

73% tests passed, 3 tests failed out of 11

Total Test time (real) = 454.77 sec

The following tests FAILED:
     2 - libclamav_valgrind (Failed)
     5 - clamscan_valgrind (Failed)
     7 - clamd_valgrind (Failed)
Errors while running CTest

We didn't have any issue building the previous version.

Is this expected now we also use rust ? Should I ignore it and remove valgrind from my Dockerfile ?

How to reproduce the problem

Using your Dockerfile and adding valgrind package. Docker build fails on all the valgrind tests.

Attachments

LastTest.log (Can provide unit test zip if needed)

micahsnyder commented 2 years ago

Hi @SecT0uch thanks for the report. Your LastTest.log attachment is perfect. It shows that valgrind is complaining about a possibly uninitialized variable deep in the Rust JPEG decoder dependency:

[INFO]: ==11503== Conditional jump or move depends on uninitialised value(s)
[INFO]: ==11503==    at 0x4FC4996: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold (mod.rs:188)
[INFO]: ==11503==    by 0x4FC03D8: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter (iterator.rs:733)
[INFO]: ==11503==    by 0x50106A7: jpeg_decoder::decoder::Decoder<R>::decode_internal (mod.rs:2486)
[INFO]: ==11503==    by 0x50AB660: <image::codecs::jpeg::decoder::JpegDecoder<R> as image::image::ImageDecoder>::read_image (decoder.rs:234)
[INFO]: ==11503==    by 0x50BDFDB: image::image::decoder_to_vec (image.rs:585)
[INFO]: ==11503==    by 0x4F72EBE: image::dynimage::DynamicImage::from_decoder (dynimage.rs:1015)
[INFO]: ==11503==    by 0x5042F2E: image::io::free_functions::load (free_functions.rs:106)
[INFO]: ==11503==    by 0x4F79935: image::dynimage::load_from_memory (dynimage.rs:1211)
[INFO]: ==11503==    by 0x4B24032: clamav_rust::fuzzy_hash::fuzzy_hash_calculate_image (fuzzy_hash.rs:412)
[INFO]: ==11503==    by 0x4B23124: fuzzy_hash_calculate_image (fuzzy_hash.rs:213)
[INFO]: ==11503==    by 0x4A1709C: calculate_fuzzy_image_hash (scanners.c:4088)
[INFO]: ==11503==    by 0x4A1C8A8: cli_magic_scan (scanners.c:4682)

And it provides a suggested suppression rule so we can ignore the alert, if that's the right call.

[INFO]: {
[INFO]:    <insert_a_suppression_name_here>
[INFO]:    Memcheck:Cond
[INFO]:    fun:_ZN102_$LT$core..iter..adapters..map..Map$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$4fold17h236ff3970b2c8ab0E
[INFO]:    fun:_ZN98_$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$alloc..vec..spec_from_iter..SpecFromIter$LT$T$C$I$GT$$GT$9from_iter17h49c93aaf09e48bf9E
[INFO]:    fun:_ZN12jpeg_decoder7decoder16Decoder$LT$R$GT$15decode_internal17h9e3bbefa711db7c5E
[INFO]:    fun:_ZN97_$LT$image..codecs..jpeg..decoder..JpegDecoder$LT$R$GT$$u20$as$u20$image..image..ImageDecoder$GT$10read_image17hf29b9de0549ff242E
[INFO]:    fun:_ZN5image5image14decoder_to_vec17h160a5286f996c471E
[INFO]:    fun:_ZN5image8dynimage12DynamicImage12from_decoder17hb3594a0e724c7d04E
[INFO]:    fun:_ZN5image2io14free_functions4load17h2489f7c909079d56E
[INFO]:    fun:_ZN5image8dynimage16load_from_memory17hba3ccd6ff2e681cfE
[INFO]:    fun:_ZN11clamav_rust10fuzzy_hash26fuzzy_hash_calculate_image17h0a75dd0dd2ceec88E
[INFO]:    fun:fuzzy_hash_calculate_image
[INFO]:    fun:calculate_fuzzy_image_hash
[INFO]:    fun:cli_magic_scan
[INFO]: }

I'm not certain if adding a suppression rule is the correct choice. I think some more triage/investigation needed to be certain. But I suspect it's not a real issue. If it is an issue, I think it'd be something with the image-rs's jpeg-decoder project https://github.com/image-rs/jpeg-decoder

SecT0uch commented 2 years ago

I will keep our production environnement pinned to Clamav 104.3 until we are 100% certain this is not a real issue. Thanks! :)