From e866c81fed39287832c2edaa7fe8cb4785519f54 Mon Sep 17 00:00:00 2001 From: Matthew Burket Date: Mon, 4 May 2026 14:14:58 -0500 Subject: [PATCH] xinetd probe: bound paths and strans keys; export oscap_path_join Use snprintf and length checks for stack buffers; build includedir paths with oscap_path_join. Mark oscap_path_join OSCAP_API for embedded tests. Add regression test for oversized name+protocol key. --- src/OVAL/probes/unix/xinetd_probe.c | 62 +++++++++++++----------- src/common/util.h | 2 +- tests/probes/xinetd/test_xinetd_probe.sh | 26 ++++++++++ 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/src/OVAL/probes/unix/xinetd_probe.c b/src/OVAL/probes/unix/xinetd_probe.c index fcb9156250..eef40b6cbd 100644 --- a/src/OVAL/probes/unix/xinetd_probe.c +++ b/src/OVAL/probes/unix/xinetd_probe.c @@ -705,10 +705,6 @@ xiconf_t *xiconf_parse(const char *path, unsigned int max_depth) #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; @@ -746,7 +742,16 @@ xiconf_t *xiconf_parse(const char *path, unsigned int max_depth) 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); @@ -756,6 +761,7 @@ xiconf_t *xiconf_parse(const char *path, unsigned int max_depth) } else break; + } case XICONF_INCTYPE_DIR: { DIR *dirfp; @@ -769,21 +775,6 @@ xiconf_t *xiconf_parse(const char *path, unsigned int max_depth) 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); @@ -800,10 +791,19 @@ xiconf_t *xiconf_parse(const char *path, unsigned int max_depth) 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) { + free(entry_path); + continue; + } + free(entry_path); } dD("includedir close: %s", inclarg); @@ -1139,10 +1139,19 @@ int xiconf_parse_section(xiconf_t *xiconf, xiconf_file_t *xifile, int type, char * 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) { @@ -1222,8 +1231,7 @@ xiconf_strans_t *xiconf_getservice(xiconf_t *xiconf, char *name, char *prot) 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); diff --git a/src/common/util.h b/src/common/util.h index 3d83fc9e43..14eaaab511 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -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); /// 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); diff --git a/tests/probes/xinetd/test_xinetd_probe.sh b/tests/probes/xinetd/test_xinetd_probe.sh index 9b35741db6..74693d5e00 100755 --- a/tests/probes/xinetd/test_xinetd_probe.sh +++ b/tests/probes/xinetd/test_xinetd_probe.sh @@ -80,6 +80,31 @@ function test_probe_xinetd_duplicates { 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 + ret_val=1 + fi + + rm -f "$tmpconf" + return $ret_val +} + # Testing. test_init @@ -88,6 +113,7 @@ if [ -z ${CUSTOM_OSCAP+x} ] ; then 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