Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

For RAR load, check LD_LIBRARY_PATH before checking install path #1258

Merged
merged 2 commits into from
May 6, 2024
Merged

For RAR load, check LD_LIBRARY_PATH before checking install path #1258

merged 2 commits into from
May 6, 2024

Conversation

val-ms
Copy link
Contributor

@val-ms val-ms commented Apr 29, 2024

ClamAV initalization's rarload() function tries to load libclamunrar_iface from the install path before checking under LD_LIBRARY_PATH.
This means the unit tests will use the wrong unrar library if testing on a system where ClamAV is already installed.
In the event there is an ABI break between versions, this will cause a bunch of tests to fail.

This commit fixes the issue by checking for libclamunrar_iface under LD_LIBRARY_PATH first before checking in the install lib directory.

Note in the previous version we were also checking LD_LIBRARY_PATH on Windows, which is not a thing. I removed this.

Fixes: #1249

Also removed check for WARN_DLOPEN_FAIL define, which was not used, and mistakenly set for the unrar library build target.

@val-ms val-ms changed the title Attemt to load RAR module from LD_LIBRARY_PATH before install path Attempt to load RAR module from LD_LIBRARY_PATH before install path Apr 29, 2024
@val-ms val-ms force-pushed the gh-1249-unrar-module-load-order branch from dd70e1e to 1f0caa2 Compare April 29, 2024 21:53
@val-ms val-ms changed the title Attempt to load RAR module from LD_LIBRARY_PATH before install path For RAR load, check LD_LIBRARY_PATH before checking install path Apr 29, 2024
TheRaynMan
TheRaynMan approved these changes Apr 29, 2024
View reviewed changes
Copy link
Contributor

@TheRaynMan TheRaynMan left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for also fixing the web link for LoadLibraryA()

@val-ms val-ms force-pushed the gh-1249-unrar-module-load-order branch from 1f0caa2 to 2676217 Compare May 1, 2024 23:57
@val-ms
For RAR load, check LD_LIBRARY_PATH before checking install path
5c6b2d6 
ClamAV initalization's rarload() function tries to load
libclamunrar_iface from the install path before checking under
LD_LIBRARY_PATH.
This means the unit tests will use the wrong unrar library if testing on
a system where ClamAV is already installed.
In the event there is an ABI break between versions, this will cause a
bunch of tests to fail.

This commit fixes the issue by checking for libclamunrar_iface under
LD_LIBRARY_PATH *first* before checking in the install lib directory.

Note in the previous version we were also checking LD_LIBRARY_PATH on
Windows, which is not a thing. I removed this.

Fixes: Cisco-Talos#1249

Also removed check for WARN_DLOPEN_FAIL define, which was not used, and
mistakenly set for the unrar library build target.
@val-ms val-ms force-pushed the gh-1249-unrar-module-load-order branch from 2676217 to 5c6b2d6 Compare May 3, 2024 14:38
@val-ms
Copy link
Contributor Author

val-ms commented May 3, 2024 •
edited
Loading

@TheRaynMan could use re-review. I tidied it up further while trying to figure out why there were test failures in Jenkins on Windows. (didn't figure it out, seemed fine to me).

Will continue to investigate.

@val-ms val-ms requested a review from TheRaynMan May 3, 2024 14:40
@val-ms val-ms force-pushed the gh-1249-unrar-module-load-order branch from d560f1d to 7502682 Compare May 3, 2024 15:00
@val-ms
Copy link
Contributor Author

val-ms commented May 3, 2024

Apparently GitHub actions on macOS fails today as well for a couple different reasons. I'm fixing them in this PR as well.

@val-ms val-ms force-pushed the gh-1249-unrar-module-load-order branch 10 times, most recently from 9f2ca94 to f6bff52 Compare May 3, 2024 20:30
@val-ms
GitHub Actions: Fix macOS build issues
68b4ab4 
Upgrade macOS OpenSSL dependency to use 3 instead of 1.1.

Python's pip from Homebrew now refuses to isntall globally:

error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try brew install
    xyz, where xyz is the package you are trying to
    install.

    If you wish to install a Python library that isn't in Homebrew,
    use a virtual environment:

    python3 -m venv path/to/venv
    source path/to/venv/bin/activate
    python3 -m pip install xyz

    If you wish to install a Python application that isn't in Homebrew,
    it may be easiest to use 'pipx install xyz', which will manage a
    virtual environment for you. You can install pipx with

    brew install pipx

    You may restore the old behavior of pip by passing
    the '--break-system-packages' flag to pip, or by adding
    'break-system-packages = true' to your pip.conf file. The latter
    will permanently disable this error.

    If you disable this error, we STRONGLY recommend that you additionally
    pass the '--user' flag to pip, or set 'user = true' in your pip.conf
    file. Failure to do this can result in a broken Homebrew installation.

    Read more about this behavior here: <https://peps.python.org/pep-0668/>

Using Pipx instead. Making the same change for Ubuntu just in case.
@val-ms val-ms force-pushed the gh-1249-unrar-module-load-order branch from f6bff52 to 68b4ab4 Compare May 3, 2024 20:36
@val-ms
Copy link
Contributor Author

val-ms commented May 3, 2024

UGH finally found the correct path for brew library installs after github's change to the macOS runners: /opt/homebrew/include and /opt/homebrew/lib.

I also fixed the windows build issues which had to do with the length of the path/commands in our Jenkins and not an actual bug in our code afaict. I solved that by relocating the build directory to be next to the source dir and to use a short directory name. Previously had been longer and in a subdirectory of the source directory.

I should be done fussing with this now.

@val-ms val-ms merged commit fa10d0c into Cisco-Talos:main May 6, 2024
21 of 24 checks passed
@val-ms val-ms deleted the gh-1249-unrar-module-load-order branch May 6, 2024 16:30
mtremer pushed a commit to ipfire/ipfire-2.x that referenced this pull request Jan 9, 2025
clamav: Update to version 1.4.1
4aae2b4 
- Update from version 1.3.2 to 1.4.1
- Update of rootfile
- Changelog
    1.4.1
	ClamAV 1.4.1 is a critical patch release with the following fixes:
	- [CVE-2024-20506](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-20506):
	  Changed the logging module to disable following symlinks on Linux and Unix
	  systems so as to prevent an attacker with existing access to the 'clamd' or
	  'freshclam' services from using a symlink to corrupt system files.
	  This issue affects all currently supported versions. It will be fixed in:
	  - 1.4.1
	  - 1.3.2
	  - 1.0.7
	  - 0.103.12
	  Thank you to Detlef for identifying this issue.
	- [CVE-2024-20505](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-20505):
	  Fixed a possible out-of-bounds read bug in the PDF file parser that could
	  cause a denial-of-service (DoS) condition.
	  This issue affects all currently supported versions. It will be fixed in:
	  - 1.4.1
	  - 1.3.2
	  - 1.0.7
	  - 0.103.12
	  Thank you to OSS-Fuzz for identifying this issue.
	- Removed unused Python modules from freshclam tests including deprecated
	  'cgi' module that is expected to cause test failures in Python 3.13.
    1.4.0
      Major changes
	- Added support for extracting ALZ archives.
	  The new ClamAV file type for ALZ archives is `CL_TYPE_ALZ`.
	  Added a [DCONF](https://docs.clamav.net/manual/Signatures/DynamicConfig.html)
	  option to enable or disable ALZ archive support.
	  > _Tip_: DCONF (Dynamic CONFiguration) is a feature that allows for some
	  > configuration changes to be made via ClamAV `.cfg` "signatures".
	  - [GitHub pull request](Cisco-Talos/clamav#1183)
	- Added support for extracting LHA/LZH archives.
	  The new ClamAV file type for LHA/LZH archives is `CL_TYPE_LHA_LZH`.
	  Added a [DCONF](https://docs.clamav.net/manual/Signatures/DynamicConfig.html)
	  option to enable or disable LHA/LZH archive support.
	  - [GitHub pull request](Cisco-Talos/clamav#1192)
	- Added the ability to disable image fuzzy hashing, if needed. For context,
	  image fuzzy hashing is a detection mechanism useful for identifying malware
	  by matching images included with the malware or phishing email/document.
	  New ClamScan options:
	  ```
	  --scan-image[=yes(*)/no]
	  --scan-image-fuzzy-hash[=yes(*)/no]
	  ```
	  New ClamD config options:
	  ```
	  ScanImage yes(*)/no
	  ScanImageFuzzyHash yes(*)/no
	  ```
	  New libclamav scan options:
	  ```c
	  options.parse &= ~CL_SCAN_PARSE_IMAGE;
	  options.parse &= ~CL_SCAN_PARSE_IMAGE_FUZZY_HASH;
	  ```
	  Added a [DCONF](https://docs.clamav.net/manual/Signatures/DynamicConfig.html)
	  option to enable or disable image fuzzy hashing support.
	  - [GitHub pull request](Cisco-Talos/clamav#1186)
      Other improvements
	- Added cross-compiling instructions for targeting ARM64/aarch64 processors for
	  [Windows](https://github.com/Cisco-Talos/clamav/blob/main/INSTALL-cross-windows-arm64.md)
	  and
	  [Linux](https://github.com/Cisco-Talos/clamav/blob/main/INSTALL-cross-linux-arm64.md).
	  - [GitHub pull request](Cisco-Talos/clamav#1116)
	- Improved the Freshclam warning messages when being blocked or rate limited
	  so as to include the Cloudflare Ray ID, which helps with issue triage.
	  - [GitHub pull request](Cisco-Talos/clamav#1195)
	- Removed unnecessary memory allocation checks when the size to be allocated
	  is fixed or comes from a trusted source.
	  We also renamed internal memory allocation functions and macros, so it is
	  more obvious what each function does.
	  - [GitHub pull request](Cisco-Talos/clamav#1137)
	- Improved the Freshclam documentation to make it clear that the `--datadir`
	  option must be an absolute path to a directory that already exists, is
	  writable by Freshclam, and is readable by ClamScan and ClamD.
	  - [GitHub pull request](Cisco-Talos/clamav#1199)
	- Added an optimization to avoid calculating the file hash if the clean file
	  cache has been disabled. The file hash may still be calculated as needed to
	  perform hash-based signature matching if any hash-based signatures exist that
	  target a file of the same size, or if any hash-based signatures exist that
	  target "any" file size.
	  - [GitHub pull request](Cisco-Talos/clamav#1167)
	- Added an improvement to the SystemD service file for ClamOnAcc so that the
	  service will shut down faster on some systems.
	  - [GitHub pull request](Cisco-Talos/clamav#1164)
	- Added a CMake build dependency on the version map files so that the build
	  will re-run if changes are made to the version map files.
	  Work courtesy of Sebastian Andrzej Siewior.
	  - [GitHub pull request](Cisco-Talos/clamav#1294)
	- Added an improvement to the CMake build so that the RUSTFLAGS settings
	  are inherited from the environment.
	  Work courtesy of liushuyu.
	  - [GitHub pull request](Cisco-Talos/clamav#1301)
      Bug fixes
	- Silenced confusing warning message when scanning some HTML files.
	  - [GitHub pull request](Cisco-Talos/clamav#1252)
	- Fixed minor compiler warnings.
	  - [GitHub pull request](Cisco-Talos/clamav#1197)
	- Since the build system changed from Autotools to CMake, ClamAV no longer
	  supports building with configurations where bzip2, libxml2, libz, libjson-c,
	  or libpcre2 are not available. Libpcre is no longer supported in favor of
	  libpcre2. In this release, we removed all the dead code associated with those
	  unsupported build configurations.
	  - [GitHub pull request](Cisco-Talos/clamav#1217)
	- Fixed assorted typos. Patch courtesy of RainRat.
	  - [GitHub pull request](Cisco-Talos/clamav#1228)
	- Added missing documentation for the ClamScan `--force-to-disk` option.
	  - [GitHub pull request](Cisco-Talos/clamav#1186)
	- Fixed an issue where ClamAV unit tests would prefer an older
	  libclamunrar_iface library from the install path, if present, rather than
	  the recently compiled library in the build path.
	  - [GitHub pull request](Cisco-Talos/clamav#1258)
	- Fixed a build issue on Windows with newer versions of Rust.
	  Also upgraded GitHub Actions imports to fix CI failures.
	  Fixes courtesy of liushuyu.
	  - [GitHub pull request](Cisco-Talos/clamav#1307)
	- Fixed an unaligned pointer dereference issue on select architectures.
	  Fix courtesy of Sebastian Andrzej Siewior.
	  - [GitHub pull request](Cisco-Talos/clamav#1293)
	- Fixed a bug that prevented loading plaintext (non-CVD) signature files
	  when using the `--fail-if-cvd-older-than=DAYS` / `FailIfCvdOlderThan` option.
	  Fix courtesy of Bark.
	  - [GitHub pull request](Cisco-Talos/clamav#1309)

Signed-off-by: Adolf Belka <[email protected]>
Signed-off-by: Arne Fitzenreiter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheRaynMan TheRaynMan TheRaynMan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

clamav 1.3.1 seven tests fail
2 participants
@val-ms @TheRaynMan