Skip to content

xinetd probe: bound paths and strans keys; export oscap_path_join #2349

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

Draft
Mab879 wants to merge 1 commit into
base: main
Choose a base branch
from
+62 −28
Draft

xinetd probe: bound paths and strans keys; export oscap_path_join #2349

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
62 changes: 35 additions & 27 deletions src/OVAL/probes/unix/xinetd_probe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,10 +705,6 @@
#define XICONF_INCTYPE_DIR 1
int inctype = -1;
char *inclarg;
char pathbuf[PATH_MAX+1];
size_t incllen;

incllen = strlen (buffer + bufidx);

if (strncmp("nclude", buffer + bufidx + 1, 6) != 0)
break;
Expand Down Expand Up @@ -746,7 +742,16 @@

switch (inctype) {
case XICONF_INCTYPE_FILE:
strncpy (pathbuf, inclarg, sizeof(pathbuf)-1);
{
char pathbuf[PATH_MAX + 1];

if (strlen(inclarg) > PATH_MAX) {
dE("includefile path too long: len=%zu, max=%zu",
strlen(inclarg), (size_t)PATH_MAX);
tmpbuf_free(buffer);
continue;
}
snprintf(pathbuf, sizeof(pathbuf), "%s", inclarg);

dD("includefile: %s", pathbuf);

Expand All @@ -756,6 +761,7 @@
}
else
break;
}
case XICONF_INCTYPE_DIR:
{
DIR *dirfp;
Expand All @@ -769,21 +775,6 @@
break;
}

strcpy (pathbuf, inclarg);
incllen = strlen(inclarg);

if (pathbuf[incllen - 1] != PATH_SEPARATOR) {
if (incllen < PATH_MAX) {
pathbuf[incllen++] = '/';
pathbuf[incllen ] = '\0';
} else {
dE("Length of the includedir argument is out of range: len=%zu, max=%zu",
incllen, PATH_MAX);
closedir(dirfp);
break;
}
}

for (;;) {
errno = 0;
dent = readdir (dirfp);
Expand All @@ -800,10 +791,19 @@
continue;
}

strcpy(pathbuf + incllen, dent->d_name);
char *entry_path = oscap_path_join(inclarg, dent->d_name);

if (xiconf_add_cfile (xiconf, pathbuf, xifile->depth + 1) != 0)
if (entry_path == NULL) {
dW("Can't build path for includedir entry: %s/%s",
inclarg, dent->d_name);
continue;
}

if (xiconf_add_cfile (xiconf, entry_path, xifile->depth + 1) != 0) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This argument to a file access function is derived from
user input (buffer read by read)
Loading
and then passed to xiconf_add_cfile(path), which calls xiconf_read(path), which calls open(__path).
free(entry_path);
continue;
}
free(entry_path);
}

dD("includedir close: %s", inclarg);
Expand Down Expand Up @@ -1139,10 +1139,19 @@
* Add entry to the ttree for (name, protocol) -> (id) translation
* (in case it's not already there)
*/
st = NULL;
strcpy(st_key, scur->name);
strcat(st_key, scur->protocol);
{
const char *st_name = scur->name != NULL ? scur->name : "";
const char *st_prot = scur->protocol != NULL ? scur->protocol : "";

if (strlen(st_name) + strlen(st_prot) > XICFG_STRANS_MAXKEYLEN) {
dE("service name+protocol too long for strans key (max %u)",
XICFG_STRANS_MAXKEYLEN);
return (-1);
}
snprintf(st_key, sizeof(st_key), "%s%s", st_name, st_prot);
}

st = NULL;
rbt_str_get(xiconf->ttree, st_key, (void *)&st);

if (st == NULL) {
Expand Down Expand Up @@ -1222,8 +1231,7 @@
if (strlen(name) + strlen(prot) > XICFG_STRANS_MAXKEYLEN)
return (NULL);

strcpy(strans_key, name);
strcat(strans_key, prot);
snprintf(strans_key, sizeof(strans_key), "%s%s", name, prot);

rbt_str_get(xiconf->ttree, strans_key, (void *)&strans);

Expand Down
2 changes: 1 addition & 1 deletion src/common/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ char *oscap_generate_random_string(size_t len, char *charset);
* @return Join of path1 and path2. The first path is separated by the second
* path by exactly 1 slash separator.
*/
char *oscap_path_join(const char *path1, const char *path2);
OSCAP_API char *oscap_path_join(const char *path1, const char *path2);
Copy link
Copy Markdown
Member

@jan-cerny jan-cerny May 7, 2026

Choose a reason for hiding this comment

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

Function prototypes marked with OSCAP_API need to be put in a public header file. Please move it for example to src/common/public/oscap.h.


/// In a list of key-value pairs (odd indicies are keys, even values), find a value for given key
const char *oscap_strlist_find_value(char ** const kvalues, const char *key);
Expand Down
26 changes: 26 additions & 0 deletions tests/probes/xinetd/test_xinetd_probe.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,31 @@
return 1
}

# Name + protocol must fit XICFG_STRANS_MAXKEYLEN (parser rejects oversize keys).
function test_probe_xinetd_strans_key_too_long {
local ret_val=0
local tmpconf
local longname

tmpconf=$(mktemp)
longname=$(printf '%260s' | tr ' ' 'a')
{
printf 'service %s\n' "$longname"
printf '{\n'
printf 'protocol = tcp\n'
printf 'socket_type = stream\n'
printf '}\n'
} >"$tmpconf"

./test_probe_xinetd "$tmpconf" ignored tcp
if [ $? -ne 2 ]; then

Check failure on line 100 in tests/probes/xinetd/test_xinetd_probe.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=AZ30cSY2PxzZiM1APt_p&open=AZ30cSY2PxzZiM1APt_p&pullRequest=2349
ret_val=1
fi

rm -f "$tmpconf"
return $ret_val
}

# Testing.

test_init
Expand All @@ -88,6 +113,7 @@
test_run "test_probe_xinetd_parser" test_probe_xinetd_parser
test_run "xinetd parser regression test: string list" test_probe_xinetd_regression_stringlist
test_run "test_probe_xinetd_duplicates" test_probe_xinetd_duplicates
test_run "test_probe_xinetd_strans_key_too_long" test_probe_xinetd_strans_key_too_long
fi

test_exit
Loading