Skip to content

Fix scan error when image fuzzy hash fails#618

Merged
val-ms merged 1 commit intoCisco-Talos:mainfrom
val-ms:CLAM-1978-fuzzy-errors
Jul 19, 2022
Merged

Fix scan error when image fuzzy hash fails#618
val-ms merged 1 commit intoCisco-Talos:mainfrom
val-ms:CLAM-1978-fuzzy-errors

Conversation

@val-ms
Copy link
Contributor

@val-ms val-ms commented Jun 23, 2022

Scanning a file containing an image where it fails to calculate the
image fuzzy hash may cause the entire scan to end with an error.

This commit fixes that by ignoring any image fuzzy hash failure.
If calculating the hash fails, the hash is left uninitialized (all
zeroes). To prevent doing image fuzzy hash lookups when it failed,
I added a boolean that is set to true only after creating the hash.

Fixes: #600
Fixes: #593

Scanning a file containing an image where it fails to calculate the
image fuzzy hash may cause the entire scan to end with an error.

This commit fixes that by ignoring any image fuzzy hash failure.
If calculating the hash fails, the hash is left uninitialized (all
zeroes). To prevent doing image fuzzy hash lookups when it failed,
I added a boolean that is set to true only after creating the hash.

Fixes: Cisco-Talos#600
Fixes: Cisco-Talos#593
Copy link
Contributor

@m-sola m-sola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During initial tests of changes, it seemed like clamscan was getting caught in an infinite loop. Further inspection via gdb shows that there's an issue with cleanup, but that it's not an infinite loop.

(gdb) run fuzzhashtest
Starting program: /home/mickey/clamav/bin/clamscan fuzzhashtest
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Loading:    19s, ETA:   0s [========================>]    8.62M/8.62M sigs
Compiling:   5s, ETA:   0s [========================>]       41/41 tasks

/home/mickey/clamav/test/fuzzhashtest: OK
^C
Program received signal SIGINT, Interrupt.
need_to_free_trans (root=<optimized out>, idx=<optimized out>) at ../libclamav/matcher-ac.c:670
670         for (j = 0; j < min; j++) {
(gdb) bt
#0  need_to_free_trans (root=<optimized out>, idx=<optimized out>) at ../libclamav/matcher-ac.c:670
#1  cli_ac_free (root=<optimized out>) at ../libclamav/matcher-ac.c:716
#2  0x00007ffff75ebbc1 in cl_engine_free (engine=0x61a000000c80) at ../libclamav/readdb.c:5674
#3  0x00000000004fbf61 in scanmanager (opts=0x6070000001e0) at ../clamscan/manager.c:1667
#4  0x00000000004fac3f in main (argc=<optimized out>, argv=<optimized out>) at ../clamscan/clamscan.c:171
(gdb) li
665         if (root->ac_nodes < idx) {
666             /*Should never happen, but check just to be safe.*/
667             min = root->ac_nodes;
668         }
669
670         for (j = 0; j < min; j++) {
671             if (NULL == root->ac_nodetable[j]) {
672                 continue;
673             }
674
(gdb) li
675             if (root->ac_nodetable[idx]->trans == root->ac_nodetable[j]->trans) {
676                 return 0;
677             }
678         }
679
680         return 1;
681     }
682
683     void cli_ac_free(struct cli_matcher *root)
684     {
(gdb) p min
$1 = 332427
(gdb) p j
$2 = 128777```

Let clamscan run for awhile to see the result (vs eicar) and found the same issue. Given the code changes, didn't seem related at all. A bit more testing (rolling back commits) confirmed that the issue was with recent AC trie changes.

The functional code changes in this commit are fine and do fix the error we've been seeing with 105 vs 103, so approving based on that.

@val-ms val-ms added the 🍒cherry-pick-candidate A PR that should be backported once approved. label Jul 15, 2022
@val-ms
Copy link
Contributor Author

val-ms commented Jul 19, 2022

During initial tests of changes, it seemed like clamscan was getting caught in an infinite loop. Further inspection via gdb shows that there's an issue with cleanup, but that it's not an infinite loop.
....
Let clamscan run for awhile to see the result (vs eicar) and found the same issue. Given the code changes, didn't seem related at all. A bit more testing (rolling back commits) confirmed that the issue was with recent AC trie changes.

AHh yup the fix for CLAM-1607 (#530) introduced an issue with really slow unload times. CLAM-1995 is the ticket to fix that. It is something we need to fix soon.

@val-ms val-ms merged commit c17f511 into Cisco-Talos:main Jul 19, 2022
@val-ms val-ms deleted the CLAM-1978-fuzzy-errors branch July 19, 2022 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🍒cherry-pick-candidate A PR that should be backported once approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Can't parse data ERROR" upon scanning simple png Some PDFs fail to scan with 0.105 (but work with 0.104)

2 participants