Skip to content

Commit

Permalink
Skip non-text files in textfilecontent54 probe
Browse files Browse the repository at this point in the history
The probe will guess whether a file is a text file and if it isn't a
text file it will skip the file. It's based on assumption that non-text
files don't contain any meaningful text to be matched by a
textfilecontent54_object. This is to filter our binary files. Up until
now binary files are processed but they cause an error produced by
pcre_exec which can't process non-UTF8 string but binary files usually
contain an invalid UTF8 string. Therefore people couldn't use it anyway.

This will prevent producing errors when you specify file path using a
regex in your OVAL content and the regex matches also non-text file paths.
For example the rule xccdf_org.ssgproject.content_rule_accounts_umask_interactive_users

Resolves: rhbz#2033246
  • Loading branch information
jan-cerny committed Mar 8, 2022
1 parent 3671701 commit 53f088d
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 1 deletion.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ find_package(DBUS)
find_package(Doxygen)
find_package(GConf)
find_package(Ldap)
find_package(Libmagic REQUIRED)
find_package(OpenDbx)
find_package(PCRE REQUIRED)
find_package(PerlLibs)
Expand Down
29 changes: 29 additions & 0 deletions cmake/FindLibmagic.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# - Try to find libmagic
# Once done, this will define
#
# LIBMAGIC_FOUND - system has libmagic
# LIBMAGIC_INCLUDE_DIRS - the libmagic include directories
# LIBMAGIC_LIBRARIES - link these to use libmagic

include(LibFindMacros)

# Use pkg-config to get hints about paths
libfind_pkg_check_modules(LIBMAGIC_PKGCONF magic-0.1)

# Include dir
find_path(LIBMAGIC_INCLUDE_DIR
NAMES magic.h
PATHS ${LIBMAGIC_PKGCONF_INCLUDE_DIRS}
)

# Finally the library itself
find_library(LIBMAGIC_LIBRARY
NAMES libmagic.so
PATHS ${LIBMAGIC_PKGCONF_LIBRARY_DIRS}
)

# Set the include dir variables and the libraries and let libfind_process do the rest.
# NOTE: Singular variables for this library, plural for libraries this this lib depends on.
set(LIBMAGIC_PROCESS_INCLUDES LIBMAGIC_INCLUDE_DIR)
set(LIBMAGIC_PROCESS_LIBS LIBMAGIC_LIBRARY)
libfind_process(LIBMAGIC)
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ set_target_properties(openscap PROPERTIES
C_VISIBILITY_PRESET hidden
)

target_link_libraries(openscap ${LIBXML2_LIBRARIES} ${LIBXSLT_LIBRARIES} ${XMLSEC_LIBRARIES} ${OPENSSL_LIBRARIES} ${LIBXSLT_EXSLT_LIBRARIES} ${PCRE_LIBRARIES} ${CURL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
target_link_libraries(openscap ${LIBXML2_LIBRARIES} ${LIBXSLT_LIBRARIES} ${XMLSEC_LIBRARIES} ${OPENSSL_LIBRARIES} ${LIBXSLT_EXSLT_LIBRARIES} ${PCRE_LIBRARIES} ${CURL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} ${LIBMAGIC_LIBRARIES})
if (BZIP2_FOUND)
target_link_libraries(openscap ${BZIP2_LIBRARIES})
endif()
Expand Down
20 changes: 20 additions & 0 deletions src/OVAL/probes/independent/textfilecontent54_probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include <unistd.h>
#include <limits.h>
#include <pcre.h>
#include <magic.h>

#include "_seap.h"
#include <probe-api.h>
Expand Down Expand Up @@ -118,6 +119,22 @@ struct pfdata {
pcre *compiled_regex;
};

static bool is_text_file(const char *filepath)
{
bool result = false;
magic_t cookie = magic_open(MAGIC_MIME);
if (cookie == NULL)
goto cleanup;
if (magic_load(cookie, NULL) != 0)
goto cleanup;
const char *magic_full = magic_file(cookie, filepath);
if (oscap_str_startswith(magic_full, "text/"))
result = true;
cleanup:
magic_close(cookie);
return result;
}

static int process_file(const char *prefix, const char *path, const char *file, void *arg, oval_schema_version_t over)
{
struct pfdata *pfd = (struct pfdata *) arg;
Expand Down Expand Up @@ -159,6 +176,9 @@ static int process_file(const char *prefix, const char *path, const char *file,
if (!S_ISREG(st.st_mode))
goto cleanup;

if (!is_text_file(whole_path_with_prefix))
goto cleanup;

fd = open(whole_path_with_prefix, O_RDONLY);
if (fd == -1) {
SEXP_t *msg;
Expand Down
1 change: 1 addition & 0 deletions tests/probes/textfilecontent54/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
if(ENABLE_PROBES_INDEPENDENT)
add_oscap_test("test_behavior_multiline.sh")
add_oscap_test("test_binary_file.sh")
add_oscap_test("test_filecontent_non_utf.sh")
add_oscap_test("test_offline_mode_textfilecontent54.sh")
add_oscap_test("test_probes_textfilecontent54.sh")
Expand Down
1 change: 1 addition & 0 deletions tests/probes/textfilecontent54/binary_file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4ċ� H���W�uw4^S������y���up�~��Q�_���ؿ �� �@\���ƞ�a ���
37 changes: 37 additions & 0 deletions tests/probes/textfilecontent54/test_binary_file.oval.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?xml version="1.0"?>
<oval_definitions xmlns:oval-def="http://oval.mitre.org/XMLSchema/oval-definitions-5" xmlns:oval="http://oval.mitre.org/XMLSchema/oval-common-5" xmlns:ind="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:ind-def="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent" xmlns:unix-def="http://oval.mitre.org/XMLSchema/oval-definitions-5#unix" xmlns:lin-def="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5" xsi:schemaLocation="http://oval.mitre.org/XMLSchema/oval-definitions-5#unix unix-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#independent independent-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#linux linux-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5 oval-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-common-5 oval-common-schema.xsd">
<generator>
<oval:schema_version>5.11.1</oval:schema_version>
<oval:timestamp>0001-01-01T00:00:00+00:00</oval:timestamp>
</generator>

<definitions>
<definition class="compliance" version="1" id="oval:x:def:1">
<metadata>
<title>A simple test OVAL for textfilecontent54 test.</title>
<description>x</description>
<affected family="unix">
<platform>x</platform>
</affected>
</metadata>
<criteria>
<criterion test_ref="oval:x:tst:1" comment="always pass"/>
</criteria>
</definition>
</definitions>

<tests>
<ind:textfilecontent54_test id="oval:x:tst:1" version="1" comment="Binary file is skipped by textfilecontent54_probe and is not matched by a regular expression" check_existence="none_exist" check="all">
<ind:object object_ref="oval:x:obj:1"/>
</ind:textfilecontent54_test>
</tests>

<objects>
<ind:textfilecontent54_object id="oval:x:obj:1" version="1" comment="Object representing file">
<ind:filepath>/tmp/binary_file</ind:filepath>
<ind:pattern operation="pattern match">^.*$</ind:pattern>
<ind:instance datatype="int" operation="greater than or equal">1</ind:instance>
</ind:textfilecontent54_object>
</objects>

</oval_definitions>
23 changes: 23 additions & 0 deletions tests/probes/textfilecontent54/test_binary_file.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/usr/bin/env bash
set -e
set -o pipefail
set -x

# Regression test for https://bugzilla.redhat.com/show_bug.cgi?id=2033246

. $builddir/tests/test_common.sh

result=$(mktemp)
stderr=$(mktemp)
cp "$srcdir/binary_file" /tmp/

$OSCAP oval eval --results "$result" "$srcdir/test_binary_file.oval.xml"

# previous versions of OpenSCAP produce result="error"
assert_exists 1 '/oval_results/results/system/definitions/definition[@definition_id="oval:x:def:1" and @result="true"]'
assert_exists 0 '/oval_results/results/system/definitions/definition[@definition_id="oval:x:def:1" and @result="error"]'
! grep "Function pcre_exec() failed to match a regular expression with return code -10 on string .*" $stderr

rm -f "$result"
rm -f "$stderr"
rm -f /tmp/binary_file

0 comments on commit 53f088d

Please sign in to comment.