Commits
rel/0.103
Name already in use
Commits on Feb 12, 2023
-
-
Disable XML entity expansion in DMG file parsing
XML entity expansion may be used to load an XML entity from a (different) local file than the file being scanned if the scanning process can read the referenced file path. This may be used to leak information from the local file to the person who initiated the scan. The libxml2 option XML_PARSE_NOENT means that no entities should be left in the document and not that no entities should be resolved. This commit removes that option.
-
Fix buffer overflow in HFS+ parser
Added buffer size test before read.
-
-
Commits on Dec 5, 2022
Commits on Jul 26, 2022
-
-
Patch UnRAR: limit dict winsize to 1GB
Prevent allocating more than 1GB regardless of what is requested. RAR dictionary sizes may not be larger than 1GB, at least in the current version. This is a cherry-pick of commit 9b444e7
-
Patch UnRAR: allow skipping files in solid archives
This is a cherry-pick of commit 24f225c Modification to unrar codebase allowing skipping of files within Solid archives when parsing in extraction mode, enabling us to skip encrypted files while still scanning metadata and potentially scanning unencrypted files later in the archive.
-
-
Fix bug with logical signature Intermediates feature
The file type check for the logical signature Intermediates feature is off-by-one and so the first layer checked is supposed to be the first parent of the current layer, but is in fact the current layer. This commit fixes the issue by correctly checking starting at -2 instead of -1. (-1 is the current layer, 0 is the outermost layer).
-
Zip parser: tolerate 2-byte overlap in file entries
The heuristic to alert on overlapping file entries is detecting some non-malicious JAR files observed in critical enterprise software. The goal with overlap detection is to alert on non-recursive zip- bombs, so this tiny overlap isn't a concern. We'll allow a 2-byte overlap so we don't alert on such zips.
Commits on May 2, 2022
Commits on Apr 29, 2022
-
Single commit to add clam mods to regex code
Changes include: * Change include of system regex headers to internal * Add cli prefix to regex functions * Change cli_regcomp to cli_regcomp_real to work with the others_common.c regex interface * Optimize re_guts struct: - Reordering fields allows the struct to fit within 16 bytes vs 20 bytes. This helps to fix a bug on legacy 64-bit systems where there was a behaviour difference between 32 and 64 systems. - see bb 474 for further details * Fix out of memory condition - see bb 849 for further details - reported by Gianluigi Tiesi <sherpya*netfarm.it> * Remove duplicate nomem check * Avoid passing out-of-range values to isalnum - reported by Nigel * Avoid name collisions on AIX * Fix compiler warnings * Fix error path leak in regex/engine.c * Fix regex when sizeof(void*) != sizeof(long) for 64bit Windows - see bb 2232 for further Details - reported by Martin Olsen * Add error case safety checks and cleanups * Add patch for 'possible' heap overflow - see bb11264 for further details - patch submitted by the Debian team * Use clam internal allocation functions * Replace bounds check asserts with if checks (asserts are compiled out of production builds) Contributors to the above include: * Nigel Horne * aCaB * Török Edvin * David Raynor * Shawn Webb * Steven Morgan * Micah Snyder * Mickey Sola
-
regex - Update internal regex to latest version
Updated using the openbsd github repo using the code in this directory: https://github.com/openbsd/src/tree/master/lib/libc/regex This build will not function without its child commit, which introduces clam specific modifications. The two have been separated to make future upgrades easier.
-
Minor type safety improvements for FMAP reads
The `fmap_readn()` function returns a size_t. On error, it returns (size_t)-1. The SIS, SWF, and TNEF parsers were storing the result as a signed int and then checking if < 0 for the error case. Added a CLI_ISCONTAINED_2_0_TO() macro, like the CLI_ISCONTAINED_0_TO() macro for use when the big buffer offset == 0, to eliminate pointless warnings. Fix enum return type for functions in SWF parser. Errors are enums, not ints! There's a difference. Fix if-check in SWF parser where we relied on integer overflow for error checking.
-
PE parser error handling, type safety, warnings
In the PE parser when reading up to 4KB following the entrypoint there's is no call to verify if the read failed. Later it is assumed that the read succeeded and that the data in the buffer is valid. I believe the correct response is to bail out if the read failed. I also fixed some warnings: - The the max # of PE sections was effectively disabled by setting it to the max size of a uint16_t, so the max-check was pointless. - Some undocumented switch fall-throughs were throwing warnings as well. - Unsigned integer subtraction results in a signed value, which was throwing warnings when compared with another unsigned value. The substraction for `peinfo->nsections - 1` won't be less than 0 though because we've already verified that nsections != 0 so we can just cast the result of the subtraction back to an unsigned value to silence the warning safely.
-
Fix possible infinite loop reading CHM archives
According to the mspack documentation: [The mspack read() callback function should] return the number of bytes successfully read (this can be less than the number requested), zero to mark the end of file, or less than zero to indicate an error. The library does not "retry" reads and assumes short reads are due to EOF, so you should avoid returning short reads because of transient errors. Our implementation returns the number of bytes read, as required, but fails to update the internal offset when reading from an fmap. This means that if mspack does retry a partial read, it will return the same bytes in perpetuity. This commit updates the offset for partial reads. It also appears as though our implementation would return more bytes than read if a partial read occurs if using a file descriptor instead of an fmap. I've corrected this to return the number of bytes read. Thank you to Michał Dardas for reporting this issue.
-
Added error checking to the TIFF parser
Thanks to Michał Dardas for reporting this.
-
Fixed memory leaks in javascript normalizer
Fixed leak where a malloced pointer was overwritten, and separate leak where a returned textbuf struct was not cleaned up. Thanks to Michał Dardas for reporting this.
-
Fix NULL param crash when caching
Since converting the hash variable from a stack array to a pointer, the pointer may now be NULL if the file is truncated after the scan starts but before the hash is calculated. This race condition would result in a NULL pointer dereference and crash. This commit adds additional NULL parameter checks. Thanks to Alexander Patrakov and Antoine Gatineau for reporting this issue. Resolves: #440
Commits on Apr 27, 2022
-
XLS parser: Fix issue where alert is lost
It is possible when not using allmatch mode that an alert on an XLS file may be "lost". That is, it first reports as FOUND but then later reports as OK. The issue is essentially that the way allmatch and alert reporting is done, it is easy to accidentally change the return value at one intermediate layer and forget about the alert. This fix doesn't clean up the design flaw, but does resolve this specific bug. Resolves: - #442 - #521
Commits on Mar 22, 2022
-
clamonacc: Added workaround for out-of-bounds array access in clamona…
…cc-ddd thread. The clamonacc-ddd thread (clamonacc/inotif/inotif.c) has currently no handling for inotify events where the passed watch descriptor is lower than zero (e.g.: "-1"). Although not actually mentioned in the inotify documentation, this case has actually been observed: [...] ClamInotif: inotify event wd:-1, mask:1073742080, cookie:0, length:16, child:<CHILD-PATH> Clamonacc: onas_clamonacc_exit(), signal 11 Assumption: This probably occurs when a inotify event is generated for the creation of a directory including subdirectories. The clamonacc-ddd thread then discovers all children of the directory and is generating inotify watch descriptors for all children. A subsequent inotify event is generated for a subdirectory, but an inotify watch descriptor already exists from the previous discovery. In any case, this leads to an out-of-bounds array access on the internal data structure "wdlt" of clamonacc while trying to look up the "path" from there. This in turn can lead to a SIGSEGV of the clamonacc-ddd thread.
-
clamonacc: Added workaround for NULL-pointer usage in clamonacc-ddd t…
…hread. The clamonacc-ddd thread (clamonacc/inotif/inotif.c) has currently no explicit handling for the special inotify kernel events IN_UNMOUNT, IN_Q_OVERFLOW and IN_IGNORED. It treats the inotify_event structs in the same way as regular inotify events, although for those special events the "len" and "name" fields a value of zero respectively NULL. This can lead to a SIGSEGV of the clamonacc-ddd thread when calling "strlen(name)" on a NULL value. This commit adds just a quick'n'dirty workaround for the SIGSEGV and some logging output. It DOES NOT fix the more overall issue, that a IN_Q_OVERFLOW inotify event also leads to an out-of-sync situation between the monitored filesystem/directory and the internal data structures "ddd_ht" and "wdlt" of clamonacc.
-
clamonacc: Added workaround for inotify watch descriptor leak in clam…
…onacc-ddd thread. The clamonacc-ddd thread (clamonacc/inotif/inotif.c) currently and in- discriminately stops processing of inotify watch descriptors in the function "onas_ddd_unwatch_hierarchy" upon any error returned from the call to "inotify_rm_watch". This in turn also prevents further updates and cleanup of the internal data structures "ddd_ht" and "wdlt", which leads to a invalid value of the "path" variable. This causes a leak of inotify watch descriptors in the case of e.g. a move of a directory containing a hierarchy of watched subdirectories. See the inotify watch descriptor ("wd:") values in the section "Example and reproducer" below. This commit adds a workaround for this issue by specifically ignoring the "ENOENT" errno in case the call to "inotify_rm_watch" results in a non-zero return code. This allows the cleanup to properly proceed for all children. Example and reproducer: - Starting point: Log: ClamInotif: created watch descriptor (wd:1) for path: /clamonacc/monitored [...] ClamInotif: created watch descriptor (wd:129637) for path: /clamonacc/monitored/[...] - Initial directory and subdirectory creation: mkdir -p /clamonacc/monitored/.test_ff/{1,2,3,4,5} Log: ClamInotif: inotify event wd:1, mask:1073742080, cookie:0, length:16, child:.test_ff ClamInotif: inotify event path: /clamonacc/monitored ClamInotif: CREATE - adding /clamonacc/monitored/.test_ff to /clamonacc/monitored with wd:1 ClamInotif: created watch descriptor (wd:129638) for path: /clamonacc/monitored/.test_ff ClamInotif: created watch descriptor (wd:129639) for path: /clamonacc/monitored/.test_ff/3 ClamInotif: created watch descriptor (wd:129640) for path: /clamonacc/monitored/.test_ff/4 ClamInotif: created watch descriptor (wd:129641) for path: /clamonacc/monitored/.test_ff/1 ClamInotif: created watch descriptor (wd:129642) for path: /clamonacc/monitored/.test_ff/5 ClamInotif: created watch descriptor (wd:129643) for path: /clamonacc/monitored/.test_ff/2 - Move / rename of the parent directory: mv -i /clamonacc/monitored/.test_ff/ /clamonacc/monitored/test_ff Log: ClamInotif: inotify event wd:1, mask:1073741888, cookie:12094045, length:16, child:.test_ff ClamInotif: inotify event path: /clamonacc/monitored ClamInotif: MOVED_FROM - removing /clamonacc/monitored/.test_ff from /clamonacc/monitored with wd:1 ERROR: ClamInotif: error removing watch descriptor (wd:129638) for path: /clamonacc/monitored/.test_ff, No such file or directory ClamInotif: inotify event wd:1, mask:1073741952, cookie:12094045, length:16, child:test_ff ClamInotif: inotify event path: /clamonacc/monitored ClamInotif: MOVED_TO - adding /clamonacc/monitored/test_ff to /clamonacc/monitored with wd:1 ClamInotif: created watch descriptor (wd:129644) for path: /clamonacc/monitored/test_ff ClamInotif: created watch descriptor (wd:129639) for path: /clamonacc/monitored/test_ff/3 ClamInotif: created watch descriptor (wd:129640) for path: /clamonacc/monitored/test_ff/4 ClamInotif: created watch descriptor (wd:129641) for path: /clamonacc/monitored/test_ff/1 ClamInotif: created watch descriptor (wd:129642) for path: /clamonacc/monitored/test_ff/5 ClamInotif: created watch descriptor (wd:129643) for path: /clamonacc/monitored/test_ff/2 - Removal of the parent directory unter the new name: rm -r /clamonacc/monitored/test_ff Log: ClamInotif: inotify event wd:129644, mask:1073742336, cookie:0, length:16, child:3 ClamInotif: inotify event path: /clamonacc/monitored/test_ff ClamInotif: DELETE - removing /clamonacc/monitored/test_ff/3 from /clamonacc/monitored/test_ff with wd:129644 ClamInotif: removed watch descriptor (wd:129639) for path: /clamonacc/monitored/test_ff/3 ClamInotif: inotify event wd:129644, mask:1073742336, cookie:0, length:16, child:4 ClamInotif: inotify event path: /clamonacc/monitored/test_ff ClamInotif: DELETE - removing /clamonacc/monitored/test_ff/4 from /clamonacc/monitored/test_ff with wd:129644 ClamInotif: removed watch descriptor (wd:129640) for path: /clamonacc/monitored/test_ff/4 ClamInotif: inotify event wd:129644, mask:1073742336, cookie:0, length:16, child:1 ClamInotif: inotify event path: /clamonacc/monitored/test_ff ClamInotif: DELETE - removing /clamonacc/monitored/test_ff/1 from /clamonacc/monitored/test_ff with wd:129644 ClamInotif: removed watch descriptor (wd:129641) for path: /clamonacc/monitored/test_ff/1 ClamInotif: inotify event wd:129644, mask:1073742336, cookie:0, length:16, child:5 ClamInotif: inotify event path: /clamonacc/monitored/test_ff ClamInotif: DELETE - removing /clamonacc/monitored/test_ff/5 from /clamonacc/monitored/test_ff with wd:129644 ClamInotif: removed watch descriptor (wd:129642) for path: /clamonacc/monitored/test_ff/5 ClamInotif: inotify event wd:129644, mask:1073742336, cookie:0, length:16, child:2 ClamInotif: inotify event path: /clamonacc/monitored/test_ff ClamInotif: DELETE - removing /clamonacc/monitored/test_ff/2 from /clamonacc/monitored/test_ff with wd:129644 ClamInotif: removed watch descriptor (wd:129643) for path: /clamonacc/monitored/test_ff/2 ClamInotif: inotify event wd:1, mask:1073742336, cookie:0, length:16, child:test_ff ClamInotif: inotify event path: /clamonacc/monitored ClamInotif: DELETE - removing /clamonacc/monitored/test_ff from /clamonacc/monitored with wd:1 ClamInotif: removed watch descriptor (wd:129644) for path: /clamonacc/monitored/test_ff -
clamonacc: Set the names of the clamonacc-ddd and clamonacc-s(can)q(u…
…eue) threads. This is also not strictly necessary, but i found it rather helpful to be able to identify and distinguish the individual clamonacc threads in the usual Linux system tools ("ps", "top", "gdb", etc.). Setting the thread name is already done for the threads started by the bundled c-thread-pool library ("thread-pool-N"). Only the clamonacc threads all identify themselfes as "clamonacc". This commit now sets the following - shortened (due to the 16 character limit) - thread names: - "clamonacc-ddd" for the ddd/inotify thread. - "clamonacc-sq" for the scan queue thread. - the main clamonacc thread currently keeps the name "clamonacc" in- herited from the main process. -
Added debug log output to bundled c-thread-pool.
This is not strictly necessary, but i found it rather helpful to know what is going on during the debugging process and while trying to under- stand how clamonacc actually works.
-
Updated bundled c-thread-pool (thpool.{c,h}) to current upstream vers…
…ion. This includes the following pull requests to the upstream c-thread-pool project, concerning the files thpool.{c,h}: - #75 (Pithikos/C-Thread-Pool#75) Commit: Pithikos/C-Thread-Pool@0e1f9e8 - #82 (Pithikos/C-Thread-Pool#82) Commit: Pithikos/C-Thread-Pool@324ddaa - #84 (Pithikos/C-Thread-Pool#84) Commit: Pithikos/C-Thread-Pool@73534f9 - #91 (Pithikos/C-Thread-Pool#91) Commit: Pithikos/C-Thread-Pool@a7cd78e - #94 (Pithikos/C-Thread-Pool#94) Commit: Pithikos/C-Thread-Pool@66fae21 - #97 (Pithikos/C-Thread-Pool#97) Commit: Pithikos/C-Thread-Pool@b259a6e -
clamonacc: Harmonize signal handling on the different clamonacc threads.
Explicit setting of pthread_sigmask in the individual threads should not be necessary at all(?), since it is already done in the main process/thread and should be inherited from there. But since the sigfillset/sigdelset lines are already there, at least they should be consistent with regard to the undefined behaviour caused by ignoring SIGFPE, SIGILL, SIGSEGV, or SIGBUS signals.
-
clamonacc: fix systemd service file
The clamonacc systemd service file includes a variable that would be filled out by CMake but not Autotools. In 0.104 we actually don't even use this option anyways as it's not required to find the default clamd config filepath. Resolves: https://bugzilla.clamav.net/show_bug.cgi?id=12761
Commits on Mar 11, 2022
-
Win32: Add missing fmap API exports
Adds missing exports for fmap functions from the clamav.h API that may be used by downstream applications. Co-authored-by: raydang <raydang@cisco.com>
Commits on Feb 16, 2022
-
Fix byte-compare subsignature premature alert
The byte compare feature in logical signatures will cause the rule to alert if it successfully matches regardless of the rest of the logical signature. An easy way to test this is with a logical signature that has two bcomp subsignatures and requires both to match for the rule to alert. In the following example, we have 4 signatures where - the first will match both bcomp subsigs. - the second will match neither. - the last two match just one bcomp subsig. In an --allmatch test, you'll find that the 3 of these match, with the first one matching *twice*, once for each bcomp subsig. test.ldb: ``` bcomp.both;Engine:51-255,Target:0;0&1&2&3;4141;0(>>5#hb2#=123);4242;2(>>5#hb2#=255) bcomp.neither;Engine:51-255,Target:0;0&1&2&3;4141;0(>>5#hb2#=124);4242;2(>>5#hb2#=254) bcomp.second;Engine:51-255,Target:0;0&1&2&3;4141;0(>>5#hb2#=124);4242;2(>>5#hb2#=255) bcomp.first;Engine:51-255,Target:0;0&1&2&3;4141;0(>>5#hb2#=123);4242;2(>>5#hb2#=254) ``` test.sample: ``` AA = 7B; BB = FF ``` You can also try a similar test to compare the behavior with regular ac-pattern-match subsigs with this lsig-test.ldb: ``` pattern.both;Engine:51-255,Target:0;0&1;4141;4242 pattern.neither;Engine:51-255,Target:0;0&1;4140;4241 pattern.second;Engine:51-255,Target:0;0&1;4140;4242 pattern.first;Engine:51-255,Target:0;0&1;4141;4241 ``` This commit fixes the issue by incrementing the logical subsignature count for each bcomp subsig match instead of appending an alert for each bcomp match. Also removed call to `lsig_sub_matched()` that didn't do anything.
-
Removed Coverity warning about null check after the value is used
Removed Coverity check by declaring subsigid as an array instead of a pointer that would need to be calloc'd, since the size is known at compile time.