Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 18 additions & 15 deletions src/OVAL/probes/independent/textfilecontent54_probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -151,23 +151,13 @@
if (probe_path_is_blocked(whole_path, blocked_paths)) {
goto cleanup;
}
/*
* If stat() fails, don't report an error and just skip the file.
* This is an expected situation, because the fts_*() functions
* are called with the 'FTS_PHYSICAL' option. Normally, stumbling
* upon a symlink without a target would cause fts_read() to return
* the 'FTS_SLNONE' flag, but the 'FTS_PHYSICAL' option causes it
* to return 'FTS_SL' and the presence of a valid target has to
* be determined with stat().
*/
whole_path_with_prefix = oscap_path_join(prefix, whole_path);
if (stat(whole_path_with_prefix, &st) == -1)
goto cleanup;
if (!S_ISREG(st.st_mode))
goto cleanup;

fd = open(whole_path_with_prefix, O_RDONLY);
fd = open(whole_path_with_prefix, O_RDONLY | O_NONBLOCK);
if (fd == -1) {
if (errno == ENOENT || errno == EACCES || errno == ENOTDIR
|| errno == ENAMETOOLONG || errno == ELOOP)
goto cleanup;

SEXP_t *msg;

msg = probe_msg_creatf(OVAL_MESSAGE_LEVEL_ERROR, "open(): '%s' %s.", whole_path, strerror(errno));
Expand All @@ -177,6 +167,19 @@
ret = -1;
goto cleanup;
}
/*
* If fstat() fails, don't report an error and just skip the file.
* This is an expected situation, because the fts_*() functions
* are called with the 'FTS_PHYSICAL' option. Normally, stumbling
* upon a symlink without a target would cause fts_read() to return
* the 'FTS_SLNONE' flag, but the 'FTS_PHYSICAL' option causes it
* to return 'FTS_SL' and the presence of a valid target has to
* be determined with fstat().
*/
if (fstat(fd, &st) == -1
|| !S_ISREG(st.st_mode)
|| probe_fd_path_is_blocked(fd, prefix, blocked_paths))
goto cleanup;

do {
buf_size += buf_inc;
Expand Down
31 changes: 16 additions & 15 deletions src/OVAL/probes/independent/textfilecontent_probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,11 @@

#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>
#include <limits.h>

#include "_seap.h"
Expand Down Expand Up @@ -139,7 +141,7 @@
static int process_file(const char *prefix, const char *path, const char *filename, void *arg, oval_schema_version_t over, struct oscap_list *blocked_paths)
{
struct pfdata *pfd = (struct pfdata *) arg;
int ret = 0, path_len, filename_len;
int ret = 0, path_len, filename_len, tmp_fd = -1;

Check warning on line 144 in src/OVAL/probes/independent/textfilecontent_probe.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define each identifier in a dedicated statement.

See more on https://sonarcloud.io/project/issues?id=OpenSCAP_openscap&issues=AaBCjOzIjq04_D56tSSS&open=AaBCjOzIjq04_D56tSSS&pullRequest=2406
char *whole_path = NULL, *whole_path_with_prefix = NULL;
FILE *fp = NULL;
struct stat st;
Expand Down Expand Up @@ -173,27 +175,24 @@
if (probe_path_is_blocked(whole_path, blocked_paths)) {
goto cleanup;
}

/*
* If stat() fails, don't report an error and just skip the file.
* This is an expected situation, because the fts_*() functions
* are called with the 'FTS_PHYSICAL' option. Normally, stumbling
* upon a symlink without a target would cause fts_read() to return
* the 'FTS_SLNONE' flag, but the 'FTS_PHYSICAL' option causes it
* to return 'FTS_SL' and the presence of a valid target has to
* be determined with stat().
*/
whole_path_with_prefix = oscap_path_join(prefix, whole_path);
if (stat(whole_path_with_prefix, &st) == -1)
tmp_fd = open(whole_path_with_prefix, O_RDONLY | O_NONBLOCK);

@Arden97 Arden97 Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@jan-cerny, @Mab879, are these warnings from github-bot legit?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think they are legit. Any users can set the OSCAP_PROBE_ROOT environment variable to any string and this string will be used as a prefix prepended to the file path, which can potentionally lead to opening arbitrary or unwanted files in the open call. However, this is a pre-existing issue, and we haven't cared so far.

if (tmp_fd == -1) {
if (errno != ENOENT && errno != EACCES && errno != ENOTDIR
&& errno != ENAMETOOLONG && errno != ELOOP)
ret = -2;
goto cleanup;
if (!S_ISREG(st.st_mode))
}
if (fstat(tmp_fd, &st) == -1
|| !S_ISREG(st.st_mode)
|| probe_fd_path_is_blocked(tmp_fd, prefix, blocked_paths))
goto cleanup;

fp = fopen(whole_path_with_prefix, "rb");
fp = fdopen(tmp_fd, "rb");
if (fp == NULL) {
ret = -2;
goto cleanup;
}
tmp_fd = -1;

int cur_inst = 0;
char line[4096];
Expand All @@ -218,6 +217,8 @@
}

cleanup:
if (tmp_fd != -1)
close(tmp_fd);
if (fp != NULL)
fclose(fp);
if (whole_path != NULL)
Expand Down
24 changes: 22 additions & 2 deletions src/OVAL/probes/independent/yamlfilecontent_probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

#include <math.h>
#include <errno.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>
#include <yaml.h>
#include <yaml-path.h>

Expand Down Expand Up @@ -390,7 +394,8 @@

static int process_yaml_file(const char *prefix, const char *path, const char *filename, const char *yamlpath, probe_ctx *ctx)
{
int ret = 0;
int ret = 0, tmp_fd = -1;

Check warning on line 397 in src/OVAL/probes/independent/yamlfilecontent_probe.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define each identifier in a dedicated statement.

See more on https://sonarcloud.io/project/issues?id=OpenSCAP_openscap&issues=AaBCjO2pjq04_D56tSST&open=AaBCjO2pjq04_D56tSST&pullRequest=2406
FILE *yaml_file = NULL;

char *filepath = oscap_path_join(path, filename);
if (probe_path_is_blocked(filepath, ctx->blocked_paths)) {
Expand All @@ -402,12 +407,25 @@
yaml_parser_initialize(&parser);

char *filepath_with_prefix = oscap_path_join(prefix, filepath);
struct stat st;

FILE *yaml_file = fopen(filepath_with_prefix, "r");
tmp_fd = open(filepath_with_prefix, O_RDONLY | O_NONBLOCK);
if (tmp_fd == -1) {
if (errno != ENOENT && errno != EACCES && errno != ENOTDIR
&& errno != ENAMETOOLONG && errno != ELOOP)
result_error("Unable to open file '%s': %s", filepath_with_prefix, strerror(errno));
goto cleanup;
}
if (fstat(tmp_fd, &st) == -1
|| !S_ISREG(st.st_mode)
|| probe_fd_path_is_blocked(tmp_fd, prefix, ctx->blocked_paths))
goto cleanup;
yaml_file = fdopen(tmp_fd, "r");
if (yaml_file == NULL) {
result_error("Unable to open file '%s': %s", filepath_with_prefix, strerror(errno));
goto cleanup;
}
tmp_fd = -1;

yaml_parser_set_input_file(&parser, yaml_file);

Expand All @@ -429,6 +447,8 @@
}

cleanup:
if (tmp_fd != -1)
close(tmp_fd);
if (yaml_file != NULL)
fclose(yaml_file);
yaml_parser_delete(&parser);
Expand Down
31 changes: 31 additions & 0 deletions src/OVAL/probes/probe-api.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
#include <arpa/inet.h> /* inet_pton() in probe_ent_from_cstr() */
#include <netinet/in.h>
#include <sys/socket.h>
#include <unistd.h>
#include <limits.h>
#endif

#include "debug_priv.h"
Expand Down Expand Up @@ -1810,4 +1812,33 @@ bool probe_path_is_blocked(const char *path, struct oscap_list *blocked_paths)
return res;
}

bool probe_fd_path_is_blocked(int fd, const char *prefix, struct oscap_list *blocked_paths)
{
#if defined(__linux__)
char proc_path[64];
char resolved[PATH_MAX];
const char *check_path;

snprintf(proc_path, sizeof(proc_path), "/proc/self/fd/%d", fd);
ssize_t len = readlink(proc_path, resolved, sizeof(resolved) - 1);
if (len == -1)
return false;
resolved[len] = '\0';
// blocked_paths are unprefixed, so we have strip prefix from check_path
check_path = resolved;
if (prefix && *prefix) {
size_t plen = strlen(prefix);
if (strncmp(resolved, prefix, plen) == 0)
check_path = resolved + plen;
}

return probe_path_is_blocked(check_path, blocked_paths);
#else
(void)fd;
(void)prefix;
(void)blocked_paths;
return false;
#endif
}

/// @}
9 changes: 9 additions & 0 deletions src/OVAL/probes/public/probe-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,4 +546,13 @@ OSCAP_API SEXP_t *probe_obj_getmask(SEXP_t *obj);
*/
OSCAP_API bool probe_path_is_blocked(const char *path, struct oscap_list *blocked_paths);

/**
* Check if the real path of an open file descriptor matches any blocked path.
* Resolves the fd target via /proc/self/fd on Linux; returns false on other platforms.
* @param fd open file descriptor
* @param prefix OSCAP_PROBE_ROOT prefix to strip from resolved path, or NULL
* @param blocked_paths list of blocked paths
*/
OSCAP_API bool probe_fd_path_is_blocked(int fd, const char *prefix, struct oscap_list *blocked_paths);

/// @}
6 changes: 6 additions & 0 deletions tests/probes/textfilecontent54/test_symlinks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
ln -s /etc/hosts ${tmpdir}/sl1
ln -s /nonexistent ${tmpdir}/sl2
ln -s ${tmpdir} ${tmpdir}/sl3
mkdir -p ${tmpdir}/blocked
echo "blocked_content" > ${tmpdir}/blocked/secret
ln -s ${tmpdir}/blocked/secret ${tmpdir}/sl4
export OSCAP_PROBE_IGNORE_PATHS="${tmpdir}/blocked"

echo "Evaluating content."
$OSCAP oval eval --results $result $input || [ $? == 2 ]
Expand All @@ -25,10 +29,12 @@
[ "$($XPATH $result 'string(/oval_results/results/system/tests/test[@test_id="oval:x:tst:2"]/@result)')" == "false" ]
[ "$($XPATH $result 'string(/oval_results/results/system/tests/test[@test_id="oval:x:tst:3"]/@result)')" == "false" ]
[ "$($XPATH $result 'string(/oval_results/results/system/tests/test[@test_id="oval:x:tst:4"]/@result)')" == "true" ]
[ "$($XPATH $result 'string(/oval_results/results/system/tests/test[@test_id="oval:x:tst:5"]/@result)')" == "false" ]

Check failure on line 32 in tests/probes/textfilecontent54/test_symlinks.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.

See more on https://sonarcloud.io/project/issues?id=OpenSCAP_openscap&issues=AaBXK4UBZ_1RHIHF_Ctq&open=AaBXK4UBZ_1RHIHF_Ctq&pullRequest=2406
echo "Testing syschar values."
[ "$($XPATH $result 'string(/oval_results/results/system/oval_system_characteristics/collected_objects/object[@id="oval:x:obj:1"]/@flag)')" == "complete" ]
[ "$($XPATH $result 'string(/oval_results/results/system/oval_system_characteristics/collected_objects/object[@id="oval:x:obj:2"]/@flag)')" == "does not exist" ]
[ "$($XPATH $result 'string(/oval_results/results/system/oval_system_characteristics/collected_objects/object[@id="oval:x:obj:3"]/@flag)')" == "does not exist" ]
[ "$($XPATH $result 'string(/oval_results/results/system/oval_system_characteristics/collected_objects/object[@id="oval:x:obj:4"]/@flag)')" == "complete" ]
[ "$($XPATH $result 'string(/oval_results/results/system/oval_system_characteristics/collected_objects/object[@id="oval:x:obj:5"]/@flag)')" == "does not exist" ]

Check failure on line 38 in tests/probes/textfilecontent54/test_symlinks.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.

See more on https://sonarcloud.io/project/issues?id=OpenSCAP_openscap&issues=AaBXK4UBZ_1RHIHF_Ctr&open=AaBXK4UBZ_1RHIHF_Ctr&pullRequest=2406

rm -rf $tmpdir
11 changes: 11 additions & 0 deletions tests/probes/textfilecontent54/test_symlinks.xml.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<criterion test_ref="oval:x:tst:2"/>
<criterion test_ref="oval:x:tst:3"/>
<criterion test_ref="oval:x:tst:4"/>
<criterion test_ref="oval:x:tst:5"/>
</criteria>
</definition>
</definitions>
Expand All @@ -36,6 +37,9 @@
<textfilecontent54_test id="oval:x:tst:4" check="all" comment="x" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent">
<object object_ref="oval:x:obj:4"/>
</textfilecontent54_test>
<textfilecontent54_test id="oval:x:tst:5" check="all" comment="x" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent">
<object object_ref="oval:x:obj:5"/>
</textfilecontent54_test>
</tests>

<objects>
Expand Down Expand Up @@ -63,5 +67,12 @@
<pattern datatype="string" operation="pattern match">.*</pattern>
<instance datatype="int" operation="equals">1</instance>
</textfilecontent54_object>
<!-- Symlink to a file under a blocked path -->
<textfilecontent54_object id="oval:x:obj:5" version="1" comment="x" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent">
<path datatype="string" operation="equals">%PATH%</path>
<filename datatype="string" operation="equals">sl4</filename>
<pattern datatype="string" operation="pattern match">.*</pattern>
<instance datatype="int" operation="equals">1</instance>
</textfilecontent54_object>
</objects>
</oval_definitions>
Loading