Clam 1796 lha lzh archive support#1192
Merged
val-ms merged 9 commits intoCisco-Talos:mainfrom Apr 9, 2024
Merged
Conversation
val-ms
force-pushed
the
CLAM-1796-lha-lzh
branch
from
6cd324f to
c639df8
Compare
Contributor
|
ctest clamscan_valgrind fails. If you can't reproduce, let me know and I'll help track it down. |
ragusaa
reviewed
Mar 7, 2024
Contributor
|
Code looks good. Need to run tests. |
val-ms
force-pushed
the
CLAM-1796-lha-lzh
branch
3 times, most recently
from
71e3b84 to
4098df4
Compare
ragusaa
reviewed
Mar 25, 2024
ragusaa
reviewed
Mar 25, 2024
Contributor
|
I added some code to scanners.rs to dump all files extracted to disk, to verify that they are the same as what 'lha' writes. I notice that when I scan the contents of lha.tar.gz, from https://jira-eng-sjc1.cisco.com/jira/browse/CLAM-2343, I do not get some of the files. The first one I notice missing is the contents of alg6.lha. Interestingly enough, if I scan only that file with clamscan, the contents are extracted. |
File type magic signatures chosen based on the extensions supported by Rust delharc crate. See: https://docs.rs/delharc/latest/delharc/
Added a test that verifies extraction of two specific files from a set of LZH files created with this utility: https://github.com/jca02266/lha
Primarily this commit fixes an issue with the size of the parameters
passed to cli_checklimits(). The parameters were "unsigned long", which
varies in size depending on platform.
I've switched them to uint64_t / u64.
While working on this, I observed some concerning warnigns on Windows,
and some less serious ones, primarily regarding inconsistencies with
`const` parameters.
Finally, in `scanmem.c`, there is a warning regarding use of `wchar_t *`
with `GetModuleFileNameEx()` instead of `GetModuleFileNameExW()`.
This made me realize this code assumes we're not defining `UNICODE`,
which would have such macros use the 'A' variant.
I have fixed it the best I can, although I'm still a little
uncomfortable with some of this code that uses `char` or `wchar_t`
instead of TCHAR.
I also remove the `if (GetModuleFileNameEx) {` conditional, because this
macro/function will always be defined. The original code was checking a
function pointer, and so this was a bug when integrating into ClamAV.
Regarding the changes to `rijndael.c`, I found that this module assumes
`unsigned long` == 32bits. It does not.
I have corrected it to use `uint32_t`.
You only need to escape % for print statements, using a second %.
No point shifting a 16bit variable more than 16 bits. Also add extra NULL terminator and inline documentation.
The delharc crate used to add LZH archive support appears to add
a dependency on macOS CoreFoundation library.
The error is:
[ 78%] Linking C shared library libclamav.dylib
Undefined symbols for architecture x86_64:
"_CFRelease", referenced from:
iana_time_zone::platform::get_timezone_inner::hc7da204717a39974 in libclamav_rust.a(iana_time_zone-bc4762a47da73d72.iana_time_zone.1863eb20d202562a-cgu.0.rcgu.o)
...
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [libclamav/libclamav.12.0.2.dylib] Error 1
We already link with CoreFoundation for libfreshclam and clamsubmit, so
this commit extends that to libclamav as well.
val-ms
force-pushed
the
CLAM-1796-lha-lzh
branch
from
decfa89 to
d52242e
Compare
val-ms
force-pushed
the
CLAM-1796-lha-lzh
branch
from
6484f76 to
b5c8624
Compare
ragusaa
approved these changes
Mar 27, 2024
val-ms
force-pushed
the
CLAM-1796-lha-lzh
branch
2 times, most recently
from
e18858d to
dbf23db
Compare
We have 2 new variations on the statx false positive, this time from calls from the Rust code. I'm removing the mangled symbol to simplify the rule and accomodate future variations.
val-ms
force-pushed
the
CLAM-1796-lha-lzh
branch
from
dbf23db to
c456824
Compare
Contributor
Author
|
I had to tweak the valgrind suppression rules a bit more, but it finally passed. I'm happy now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR still requires some unit tests.