Skip to content

Prevent denial of service in filehash and filehash58 probes #2407

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

Open
jan-cerny wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Prevent denial of service in filehash and filehash58 probes #2407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions src/OVAL/probes/independent/filehash58_probe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,16 @@

/*
* Open the file
*
* O_NONBLOCK ensures that open() does not block indefinitely when the
* target is a FIFO (named pipe) with no writer, or another special file
* whose open can block. For regular files O_NONBLOCK has no effect.
*/
if (prefix == NULL) {
fd = open(pbuf, O_RDONLY);
fd = open(pbuf, O_RDONLY | O_NONBLOCK);
} else {
char *path_with_prefix = oscap_path_join(prefix, pbuf);
fd = open(path_with_prefix, O_RDONLY);
fd = open(path_with_prefix, O_RDONLY | O_NONBLOCK);
free(path_with_prefix);
}

Expand All @@ -186,6 +190,45 @@
return 0;
}

/*
* Make sure we only try to hash regular files. Hashing a FIFO, socket,
* device or other special file makes no sense and could block the read.
* The type is checked on the already-opened descriptor (fstat) to avoid
* a TOCTOU race with the preceding open().
*/
struct stat st;
if (fstat(fd, &st) != 0) {
char errbuf[__ERRBUF_SIZE] = {0};
oscap_strerror_r(errno, errbuf, sizeof errbuf - 1);
itm = probe_item_create(OVAL_INDEPENDENT_FILE_HASH58, NULL,
"filepath", OVAL_DATATYPE_STRING, pbuf,
"path", OVAL_DATATYPE_STRING, p,
"filename", OVAL_DATATYPE_STRING, f,
"hash_type",OVAL_DATATYPE_STRING, h,
NULL);
probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR,
"File \"%s\" status can't be read: %s", pbuf, errbuf);
probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR);

probe_item_collect(ctx, itm);
close(fd);
return 0;
} else if (!S_ISREG(st.st_mode)) {
itm = probe_item_create(OVAL_INDEPENDENT_FILE_HASH58, NULL,
"filepath", OVAL_DATATYPE_STRING, pbuf,
"path", OVAL_DATATYPE_STRING, p,
"filename", OVAL_DATATYPE_STRING, f,
"hash_type",OVAL_DATATYPE_STRING, h,
NULL);
probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR,
"File \"%s\" is not a regular file.", pbuf);
probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR);

probe_item_collect(ctx, itm);
close(fd);
return 0;
}

uint8_t hash_dst[1025];
size_t hash_dstlen = sizeof(hash_dst);
char hash_str[2051];
Expand Down
47 changes: 44 additions & 3 deletions src/OVAL/probes/independent/filehash_probe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "filehash_probe.h"

#define FILE_SEPARATOR '/'
#define __ERRBUF_SIZE 128

static int mem2hex (uint8_t *mem, size_t mlen, char *str, size_t slen)
{
Expand All @@ -74,7 +75,7 @@
return (0);
}

static int filehash_cb (const char *prefix, const char *p, const char *f, probe_ctx *ctx, oval_schema_version_t over)

Check failure on line 78 in src/OVAL/probes/independent/filehash_probe.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 30 to the 25 allowed. 


See more on https://sonarcloud.io/project/issues?id=OpenSCAP_openscap&issues=AaBYEx2Dte7fXHyrmR3V&open=AaBYEx2Dte7fXHyrmR3V&pullRequest=2407
{
SEXP_t *itm;
char pbuf[PATH_MAX+1];
Expand Down Expand Up @@ -111,17 +112,20 @@

/*
* Open the file
*
* O_NONBLOCK ensures that open() does not block indefinitely when the
* target is a FIFO (named pipe) with no writer, or another special file
* whose open can block. For regular files O_NONBLOCK has no effect.
*/
if (prefix == NULL) {
fd = open(pbuf, O_RDONLY);
fd = open(pbuf, O_RDONLY | O_NONBLOCK);
} else {
char *path_with_prefix = oscap_path_join(prefix, pbuf);
fd = open(path_with_prefix, O_RDONLY);
fd = open(path_with_prefix, O_RDONLY | O_NONBLOCK);
free(path_with_prefix);
}

if (fd < 0) {
#define __ERRBUF_SIZE 128
char errbuf[__ERRBUF_SIZE] = {0};
oscap_strerror_r(errno, errbuf, sizeof errbuf - 1);

Expand All @@ -136,6 +140,43 @@
probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR);

} else {
/*
* Make sure we only try to hash regular files. Hashing a FIFO,
* socket, device or other special file makes no sense and could
* block the read. The type is checked on the already-opened
* descriptor (fstat) to avoid a TOCTOU race with the open().
*/
struct stat st;
if (fstat(fd, &st) != 0) {
char errbuf[__ERRBUF_SIZE] = {0};
oscap_strerror_r(errno, errbuf, sizeof errbuf - 1);
itm = probe_item_create(OVAL_INDEPENDENT_FILE_HASH, NULL,
"filepath", OVAL_DATATYPE_STRING, include_filepath ? pbuf : NULL,
"path", OVAL_DATATYPE_STRING, p,
"filename", OVAL_DATATYPE_STRING, f,
NULL);
probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR,
"File \"%s\" status can't be read: %s", pbuf, errbuf);
probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR);

probe_item_collect(ctx, itm);
close(fd);
return 0;
} else if (!S_ISREG(st.st_mode)) {
itm = probe_item_create(OVAL_INDEPENDENT_FILE_HASH, NULL,
"filepath", OVAL_DATATYPE_STRING, include_filepath ? pbuf : NULL,
"path", OVAL_DATATYPE_STRING, p,
"filename", OVAL_DATATYPE_STRING, f,
NULL);
probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR,
"File \"%s\" is not a regular file.", pbuf);
probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR);

probe_item_collect(ctx, itm);
close(fd);
return 0;
}

uint8_t md5_dst[16];
size_t md5_dstlen = sizeof md5_dst;
char md5_str[(sizeof md5_dst * 2) + 1];
Expand Down
1 change: 1 addition & 0 deletions tests/probes/filehash/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
if(OPENSCAP_PROBE_INDEPENDENT_FILEHASH)
add_oscap_test("test_probes_filehash.sh" LABELS independent)
add_oscap_test("test_probes_filehash_dos.sh" LABELS independent)
endif()
40 changes: 40 additions & 0 deletions tests/probes/filehash/dos.oval.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<ns0:oval_definitions xmlns:ns0="http://oval.mitre.org/XMLSchema/oval-definitions-5" xmlns:ns2="http://oval.mitre.org/XMLSchema/oval-common-5" xmlns:ns3="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent" xmlns:ns4="http://oval.mitre.org/XMLSchema/oval-definitions-5#unix" xmlns:ns5="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://oval.mitre.org/XMLSchema/oval-common-5 oval-common-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5 oval-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#independent independent-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#unix unix-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#linux linux-definitions-schema.xsd">
<ns0:generator>
<ns2:product_name>combine_ovals.py from SCAP Security Guide</ns2:product_name>
<ns2:product_version>ssg: [0, 1, 40], python: 3.6.5</ns2:product_version>
<ns2:schema_version>5.11</ns2:schema_version>
<ns2:timestamp>2018-07-20T09:33:24</ns2:timestamp>
</ns0:generator>
<ns0:definitions>
<ns0:definition class="compliance" id="oval:ssg-oval_test_has_hash:def:1" version="1">
<ns0:metadata>
<ns0:title>Test that reading a FIFO (named pipe) by the filehash probe doesn't freeze OpenSCAP.</ns0:title>
<ns0:affected family="unix">
<ns0:platform>Red Hat Enterprise Linux 7</ns0:platform>
</ns0:affected>
<ns0:description>description</ns0:description>
<reference ref_id="oval_test_has_hash" source="ssg" />
</ns0:metadata>
<ns0:criteria>
<ns0:criterion comment="Check file hash of /oval-test" test_ref="oval:xxx:tst:1" />
<ns0:criterion comment="Check file hash of /oval-test" test_ref="oval:xxx:tst:2" />
</ns0:criteria>
</ns0:definition>
</ns0:definitions>
<ns0:tests>
<ns3:filehash_test check="all" comment="-" id="oval:xxx:tst:1" version="1">
<ns3:object object_ref="oval:xxx:obj:1" />
</ns3:filehash_test>
<ns3:filehash_test check="all" comment="-" id="oval:xxx:tst:2" version="1">
<ns3:object object_ref="oval:xxx:obj:2" />
</ns3:filehash_test>
</ns0:tests>
<ns0:objects>
<ns3:filehash_object id="oval:xxx:obj:1" version="1">
<ns3:filepath>/tmp/oscap-test-fifo</ns3:filepath>
</ns3:filehash_object>
<ns3:filehash_object id="oval:xxx:obj:2" version="1">
<ns3:filepath>/tmp/oscap-test-symlink</ns3:filepath>
</ns3:filehash_object>
</ns0:objects>
</ns0:oval_definitions>
36 changes: 36 additions & 0 deletions tests/probes/filehash/test_probes_filehash_dos.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env bash

# Copyright 2021 Red Hat Inc., Durham, North Carolina.
# All Rights Reserved.
#
# OpenSCAP Probes Test Suite.
#
# Authors:
# Jan Černý, <jcerny@redhat.com>

set -e -o pipefail
set -x
. $builddir/tests/test_common.sh

probecheck "filehash" || return 255
require "md5sum" || return 255
require "sha1sum" || return 255

# Block the scan indefinitely
rm -f /tmp/oscap-test-fifo
mkfifo /tmp/oscap-test-fifo

# Burn CPU forever
rm -f /tmp/oscap-test-symlink
ln -s /dev/zero /tmp/oscap-test-symlink

result="$(mktemp)"

$OSCAP oval eval --results "$result" "$srcdir/dos.oval.xml"
[[ $? -eq 0 ]]
assert_exists 1 '/oval_results/results/system/tests/test[@test_id="oval:xxx:tst:1"][@result="error"]'
assert_exists 1 '/oval_results/results/system/tests/test[@test_id="oval:xxx:tst:2"][@result="error"]'

rm -f /tmp/oscap-test-fifo
rm -f /tmp/oscap-test-symlink
rm -f "$result"
1 change: 1 addition & 0 deletions tests/probes/filehash58/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
if(ENABLE_PROBES_INDEPENDENT)
add_oscap_test("test_probes_filehash58.sh" LABELS independent)
add_oscap_test("test_probes_filehash58_dos.sh" LABELS independent)
add_oscap_test("rhbz1959570_segfault.sh" LABELS independent)
endif()
42 changes: 42 additions & 0 deletions tests/probes/filehash58/dos.oval.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<ns0:oval_definitions xmlns:ns0="http://oval.mitre.org/XMLSchema/oval-definitions-5" xmlns:ns2="http://oval.mitre.org/XMLSchema/oval-common-5" xmlns:ns3="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent" xmlns:ns4="http://oval.mitre.org/XMLSchema/oval-definitions-5#unix" xmlns:ns5="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://oval.mitre.org/XMLSchema/oval-common-5 oval-common-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5 oval-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#independent independent-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#unix unix-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#linux linux-definitions-schema.xsd">
<ns0:generator>
<ns2:product_name>combine_ovals.py from SCAP Security Guide</ns2:product_name>
<ns2:product_version>ssg: [0, 1, 40], python: 3.6.5</ns2:product_version>
<ns2:schema_version>5.11</ns2:schema_version>
<ns2:timestamp>2018-07-20T09:33:24</ns2:timestamp>
</ns0:generator>
<ns0:definitions>
<ns0:definition class="compliance" id="oval:ssg-oval_test_has_hash:def:1" version="1">
<ns0:metadata>
<ns0:title>Test that reading a FIFO (named pipe) by the filehash58 probe doesn't freeze OpenSCAP.</ns0:title>
<ns0:affected family="unix">
<ns0:platform>Red Hat Enterprise Linux 7</ns0:platform>
</ns0:affected>
<ns0:description>description</ns0:description>
<reference ref_id="oval_test_has_hash" source="ssg" />
</ns0:metadata>
<ns0:criteria>
<ns0:criterion comment="Check file hash of /oval-test" test_ref="oval:xxx:tst:1" />
<ns0:criterion comment="Check file hash of /oval-test" test_ref="oval:xxx:tst:2" />
</ns0:criteria>
</ns0:definition>
</ns0:definitions>
<ns0:tests>
<ns3:filehash58_test check="all" comment="-" id="oval:xxx:tst:1" version="1">
<ns3:object object_ref="oval:xxx:obj:1" />
</ns3:filehash58_test>
<ns3:filehash58_test check="all" comment="-" id="oval:xxx:tst:2" version="1">
<ns3:object object_ref="oval:xxx:obj:2" />
</ns3:filehash58_test>
</ns0:tests>
<ns0:objects>
<ns3:filehash58_object id="oval:xxx:obj:1" version="1">
<ns3:filepath>/tmp/oscap-test-fifo</ns3:filepath>
<ns3:hash_type>SHA-256</ns3:hash_type>
</ns3:filehash58_object>
<ns3:filehash58_object id="oval:xxx:obj:2" version="1">
<ns3:filepath>/tmp/oscap-test-symlink</ns3:filepath>
<ns3:hash_type>SHA-256</ns3:hash_type>
</ns3:filehash58_object>
</ns0:objects>
</ns0:oval_definitions>
37 changes: 37 additions & 0 deletions tests/probes/filehash58/test_probes_filehash58_dos.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/usr/bin/env bash

# Copyright 2021 Red Hat Inc., Durham, North Carolina.
# All Rights Reserved.
#
# OpenSCAP Probes Test Suite.
#
# Authors:
# Jan Černý, <jcerny@redhat.com>

set -e -o pipefail
set -x
. $builddir/tests/test_common.sh

probecheck "filehash58" || return 255
require "sha256sum" || return 255
require "sha512sum" || return 255

# Block the scan indefinitely
rm -f /tmp/oscap-test-fifo
mkfifo /tmp/oscap-test-fifo

# Burn CPU forever
rm -f /tmp/oscap-test-symlink
ln -s /dev/zero /tmp/oscap-test-symlink

result="$(mktemp)"

$OSCAP oval eval --results "$result" "$srcdir/dos.oval.xml"
[[ $? -eq 0 ]]
assert_exists 1 '/oval_results/results/system/tests/test[@test_id="oval:xxx:tst:1"][@result="error"]'
assert_exists 1 '/oval_results/results/system/tests/test[@test_id="oval:xxx:tst:2"][@result="error"]'

rm -f /tmp/oscap-test-fifo
rm -f /tmp/oscap-test-symlink
rm -f "$result"

Loading