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

Refuse to map procfs files on Linux #1848

Merged
merged 1 commit into from
Jan 2, 2023
Merged

Refuse to map procfs files on Linux #1848

merged 1 commit into from
Jan 2, 2023

Conversation

hillu
Copy link
Contributor

@hillu hillu commented Dec 31, 2022

It makes no sense to try to mmap files for which the filesystem lies about the size (0).

Clsoe #1838

(It may make sense to consider other pseudo filesystems on Linux and other Unices as well.)

@hillu
Refuse to map procfs files on Linux
fb9ee2e 
It makes no sense to try to mmap files for which the filesystem lies
about the size (0).

Clsoe VirusTotal#1838
@plusvic plusvic merged commit b5a0cfd into VirusTotal:master Jan 2, 2023
@plusvic plusvic mentioned this pull request Jan 2, 2023
Closed
fengjixuchui added a commit to fengjixuchui/yara that referenced this pull request Jan 2, 2023
@fengjixuchui
Merge pull request #4 from VirusTotal/master
dc44f75 
Refuse to map procfs files on Linux (VirusTotal#1848)
@ikrivosheev ikrivosheev mentioned this pull request Apr 24, 2023
Closed
@gwillem
Copy link

gwillem commented Sep 3, 2024 •
edited
Loading

A dependency to libmagick, linux/magic.h complicates cross compilation. @hillu how did you fix this for your own projects?

@hillu
Copy link
Contributor Author

hillu commented Sep 4, 2024 •
edited
Loading

A dependency to libmagick, linux/magic.h

No, linux/magic.h has nothing to do with libmagick; on Debian and derivative distributions, it is shipped as part of linux-libc-dev.

"magic" in this case refers to filesystem magic numbers reported by fstatfs, we only need PROC_SUPER_MAGIC which is part of the Kernel API (and thus not going to change). So we could get away with this patch:

--- a/libyara/filemap.c
+++ b/libyara/filemap.c
@@ -159,7 +159,7 @@ YR_API int yr_filemap_map_fd(
 
 #ifdef __linux__
 #include <sys/vfs.h>
-#include <linux/magic.h>
+#define PROC_SUPER_MAGIC       0x9fa0
 #endif
 
 #define MAP_EXTRA_FLAGS 0

@gwillem
Copy link

gwillem commented Sep 4, 2024

Thanks a lot Hillu! Somehow the Go linker keeps adding -lmagic to my build which I assumed was related to the above but I should continue my search. Anyhow, thanks for the patch!

@plusvic
Copy link
Member

plusvic commented Sep 4, 2024

There's some confusion here. This PR was not about libmagic, but @gwillem's has unrelated issue with libmagic and landed here, probably after searching for the "magic" keyword.

@gwillem what's the output of pkg-config --libs yara in your system? It's something like -L/usr/local/lib -lyara -lmagic?

@gwillem
Copy link

gwillem commented Sep 5, 2024 •
edited
Loading

Thanks @plusvic. To clarify, there were two issues:

  1. Dep on magic.h (see also Failing to compile with go-yara hillu/go-yara#147 (comment)), solved by Hillu's patch above.

  2. Unknown origin of -lmagic injection by the Go linker. This was introduced by our upgrade of yara 4.0.2 to 4.3.2. However, we just upgraded to 4.5.1 and our build succeeds. Apologies for reporting an issue with an older version 😅

(We use https://github.com/hillu/go-yara v4.3 and we incorrectly assumed we had to use a corresponding yara version)

hillu added a commit to hillu/yara that referenced this pull request Sep 5, 2024
@hillu
filemap: define PROC_SUPER_MAGIC, avoid linux/magic.h
509fc7c 
PR VirusTotal#1848 caused build issues with some "unusual" build configurations
– apparently we can't rely on linux/magic.h being present when
cross-building for musl libc.

Defining PROC_SUPER_MAGIC should not cause a problems since it should
be considered part of the Linux kernel/user API and it is unlikely to
change.
@hillu hillu mentioned this pull request Sep 5, 2024
Merged
hillu added a commit to hillu/yara that referenced this pull request Sep 5, 2024
@hillu
filemap: define PROC_SUPER_MAGIC, avoid linux/magic.h
f52c1f0 
PR VirusTotal#1848 caused build issues with some "unusual" build configurations
– apparently we can't rely on linux/magic.h being present when
cross-building for musl libc.

Defining PROC_SUPER_MAGIC should not cause a problems since it should
be considered part of the Linux kernel/user API and it is unlikely to
change.
plusvic pushed a commit that referenced this pull request Sep 5, 2024
@hillu
filemap: define PROC_SUPER_MAGIC, avoid linux/magic.h (#2103)
5194025 
PR #1848 caused build issues with some "unusual" build configurations
– apparently we can't rely on linux/magic.h being present when
cross-building for musl libc.

Defining PROC_SUPER_MAGIC should not cause a problems since it should
be considered part of the Linux kernel/user API and it is unlikely to
change.
DavidTurland pushed a commit to DavidTurland/yara that referenced this pull request Sep 9, 2024
@hillu @DavidTurland
filemap: define PROC_SUPER_MAGIC, avoid linux/magic.h (VirusTotal#2103)
32afae0 
PR VirusTotal#1848 caused build issues with some "unusual" build configurations
– apparently we can't rely on linux/magic.h being present when
cross-building for musl libc.

Defining PROC_SUPER_MAGIC should not cause a problems since it should
be considered part of the Linux kernel/user API and it is unlikely to
change.
DavidTurland added a commit to DavidTurland/yara that referenced this pull request Sep 9, 2024
@DavidTurland @plusvic
@carceneaux @chrisdoman @wesinator @GLMONTER @hillu
Merge from VT master (#6)
3832caf 
* Fix crash while parsing PE Rich header

File e77b007c9a964411c5e33afeec18be32c86963b78f3c3e906b28fcf1382f46c3 has a Rich header of only 8 bytes, which is smaller than the RICH_SIGNATURE structure. This was causing a crash when some of the `rich_xxx` functions were used with this file.

* Fix warning

`_rich_version` in PE module should return an `int64_t` instead of `uint64_t`.

* Use YR_MAX_PATH instead of MAX_PATH (VirusTotal#2090)

Replace all instances of `MAX_PATH` with `YR_MAX_PATH`.

* Adding Veeam (VirusTotal#2083)

Adding Veeam to list of companies that use YARA.

* Add Cado to who is using Yara (VirusTotal#2086)

* Mitigate stack overflow when scanning very deep directory trees.

Closes VirusTotal#2088.

* Remove all references to ERROR_TOO_MANY_SCAN_THREADS

This error code is not used anymore. Closes VirusTotal#2068.

* Use latest MacOS in build workflow.

* Use MacOS 13 in build workflow.

For some reason in MacOS 14 the build fails because the `configure` script is unable to find the Jansson library, even thought it is correctly installed by `brew`.

* docs: minor updates to xor (VirusTotal#2098)

* use new module macros in docs (VirusTotal#2100)

Co-authored-by: Tad Keller <logisch@pm.me>

* filemap: define PROC_SUPER_MAGIC, avoid linux/magic.h (VirusTotal#2103)

PR VirusTotal#1848 caused build issues with some "unusual" build configurations
– apparently we can't rely on linux/magic.h being present when
cross-building for musl libc.

Defining PROC_SUPER_MAGIC should not cause a problems since it should
be considered part of the Linux kernel/user API and it is unlikely to
change.

---------

Co-authored-by: Victor M. Alvarez <vmalvarez@virustotal.com>
Co-authored-by: Chris Arceneaux <carcenea@gmail.com>
Co-authored-by: chrisdoman <chris.doman@cantab.net>
Co-authored-by: Wes <5124946+wesinator@users.noreply.github.com>
Co-authored-by: Tad Keller <43346260+GLMONTER@users.noreply.github.com>
Co-authored-by: Tad Keller <logisch@pm.me>
Co-authored-by: Hilko Bengen <bengen@hilluzination.de>
DavidTurland added a commit to DavidTurland/yara that referenced this pull request Sep 9, 2024
@DavidTurland @plusvic
@carceneaux @chrisdoman @wesinator @GLMONTER @hillu
Merge from VT (#7)
c199108 
* Fix crash while parsing PE Rich header

File e77b007c9a964411c5e33afeec18be32c86963b78f3c3e906b28fcf1382f46c3 has a Rich header of only 8 bytes, which is smaller than the RICH_SIGNATURE structure. This was causing a crash when some of the `rich_xxx` functions were used with this file.

* Fix warning

`_rich_version` in PE module should return an `int64_t` instead of `uint64_t`.

* Use YR_MAX_PATH instead of MAX_PATH (VirusTotal#2090)

Replace all instances of `MAX_PATH` with `YR_MAX_PATH`.

* Adding Veeam (VirusTotal#2083)

Adding Veeam to list of companies that use YARA.

* Add Cado to who is using Yara (VirusTotal#2086)

* Mitigate stack overflow when scanning very deep directory trees.

Closes VirusTotal#2088.

* Remove all references to ERROR_TOO_MANY_SCAN_THREADS

This error code is not used anymore. Closes VirusTotal#2068.

* Use latest MacOS in build workflow.

* Use MacOS 13 in build workflow.

For some reason in MacOS 14 the build fails because the `configure` script is unable to find the Jansson library, even thought it is correctly installed by `brew`.

* docs: minor updates to xor (VirusTotal#2098)

* use new module macros in docs (VirusTotal#2100)

Co-authored-by: Tad Keller <logisch@pm.me>

* filemap: define PROC_SUPER_MAGIC, avoid linux/magic.h (VirusTotal#2103)

PR VirusTotal#1848 caused build issues with some "unusual" build configurations
– apparently we can't rely on linux/magic.h being present when
cross-building for musl libc.

Defining PROC_SUPER_MAGIC should not cause a problems since it should
be considered part of the Linux kernel/user API and it is unlikely to
change.

---------

Co-authored-by: Victor M. Alvarez <vmalvarez@virustotal.com>
Co-authored-by: Chris Arceneaux <carcenea@gmail.com>
Co-authored-by: chrisdoman <chris.doman@cantab.net>
Co-authored-by: Wes <5124946+wesinator@users.noreply.github.com>
Co-authored-by: Tad Keller <43346260+GLMONTER@users.noreply.github.com>
Co-authored-by: Tad Keller <logisch@pm.me>
Co-authored-by: Hilko Bengen <bengen@hilluzination.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plusvic plusvic plusvic 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.
3 participants
@hillu @gwillem @plusvic