diff --git a/cf-agent/verify_files_utils.c b/cf-agent/verify_files_utils.c index 6b819922a68..3ce62cfb8d0 100644 --- a/cf-agent/verify_files_utils.c +++ b/cf-agent/verify_files_utils.c @@ -1335,16 +1335,26 @@ static PromiseResult LinkCopy(EvalContext *ctx, char *sourcefile, char *destfile strlcpy(vbuff, changes_sourcefile, CF_BUFSIZE); ChopLastNode(vbuff); AddSlash(vbuff); - strncat(vbuff, chrooted_linkbuf, CF_BUFSIZE - 1); - strlcpy(chrooted_linkbuf, vbuff, CF_BUFSIZE - 1); + if (strlcat(vbuff, chrooted_linkbuf, sizeof(vbuff)) >= sizeof(vbuff) || + strlcpy(chrooted_linkbuf, vbuff, sizeof(chrooted_linkbuf)) >= sizeof(chrooted_linkbuf)) + { + RecordFailure(ctx, pp, attr, + "Absolute target of link '%s' is too long", sourcefile); + return PROMISE_RESULT_FAIL; + } } else { strlcpy(vbuff, sourcefile, CF_BUFSIZE); ChopLastNode(vbuff); AddSlash(vbuff); - strncat(vbuff, linkbuf, CF_BUFSIZE - 1); - strlcpy(linkbuf, vbuff, CF_BUFSIZE - 1); + if (strlcat(vbuff, linkbuf, sizeof(vbuff)) >= sizeof(vbuff) || + strlcpy(linkbuf, vbuff, sizeof(linkbuf)) >= sizeof(linkbuf)) + { + RecordFailure(ctx, pp, attr, + "Absolute target of link '%s' is too long", sourcefile); + return PROMISE_RESULT_FAIL; + } } } } diff --git a/libpromises/files_links.c b/libpromises/files_links.c index afcabc3e158..ec4762abb07 100644 --- a/libpromises/files_links.c +++ b/libpromises/files_links.c @@ -536,7 +536,13 @@ bool KillGhostLink(EvalContext *ctx, const char *name, const Attributes *attr, c } } - strcat(linkpath, linkbuf); + if (strlcat(linkpath, linkbuf, sizeof(linkpath)) >= sizeof(linkpath)) + { + Log(LOG_LEVEL_ERR, + "Internal limit reached in KillGhostLink()," + " path too long: '%s' + '%s'", linkpath, linkbuf); + return false; + } CompressPath(tmp, sizeof(tmp), linkpath); if (stat(tmp, &statbuf) == -1) /* link points nowhere */ @@ -715,8 +721,17 @@ bool ExpandLinks(char *dest, const char *from, int level, int max_level) continue; } - sscanf(sp, "%[^/]", node); - sp += strlen(node); + size_t node_len = strcspn(sp, "/"); + if (node_len >= sizeof(node)) + { + Log(LOG_LEVEL_ERR, + "Internal limit reached in ExpandLinks()," + " path component too long: '%s'", sp); + return false; + } + memcpy(node, sp, node_len); + node[node_len] = '\0'; + sp += node_len; if (*sp == '\0') { diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 388bb4cecb9..505ade7eaf0 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -120,6 +120,7 @@ check_PROGRAMS = \ package_versions_compare_test \ files_lib_test \ files_copy_test \ + files_links_test \ parsemode_test \ parser_test \ passopenfile_test \ @@ -300,6 +301,9 @@ package_versions_compare_test_LDADD = ../../libpromises/libpromises.la \ files_copy_test_SOURCES = files_copy_test.c files_copy_test_LDADD = libtest.la ../../libpromises/libpromises.la +files_links_test_SOURCES = files_links_test.c +files_links_test_LDADD = libtest.la ../../libpromises/libpromises.la + sort_test_SOURCES = sort_test.c sort_test_LDADD = libtest.la ../../libpromises/libpromises.la diff --git a/tests/unit/files_links_test.c b/tests/unit/files_links_test.c new file mode 100644 index 00000000000..0b1eefc73c6 --- /dev/null +++ b/tests/unit/files_links_test.c @@ -0,0 +1,95 @@ +/* + Copyright 2026 Northern.tech AS + + This file is part of CFEngine 3 - written and maintained by Northern.tech AS. + + This program is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by the + Free Software Foundation; version 3. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA + + To the extent this program is licensed as part of the Enterprise + versions of CFEngine, the applicable Commercial Open Source License + (COSL) may apply to this file if you as a licensee so wish it. See + included file COSL.txt. +*/ + +#include + +#include +#include /* ExpandLinks, KillGhostLink */ +#include /* Attributes */ +#include /* symlink, unlink */ +#include /* errno */ + +/* A single path component longer than the internal node buffer + * (CF_MAXLINKSIZE) overflowed the stack in ExpandLinks(): the parse into + * 'node' ran before any lstat(), so a symlink target with a long slash-free + * component smashed the stack. It must bail out instead. */ +static void test_expand_links_long_component(void) +{ + char dest[CF_BUFSIZE]; + + char from[1024]; + from[0] = '/'; + memset(from + 1, 'a', sizeof(from) - 2); + from[sizeof(from) - 1] = '\0'; + + bool ret = ExpandLinks(dest, from, 0, CF_MAXLINKLEVEL); + assert_false(ret); +} + +/* KillGhostLink() rebuilds the absolute target of a relative symlink by + * appending the readlink() result onto the link's directory in a CF_BUFSIZE + * buffer. The append was an unbounded strcat(), so a symlink whose relative + * target is near CF_BUFSIZE overflowed the stack. It must bail out instead. */ +static void test_kill_ghost_link_long_target(void) +{ + char tmpl[] = "/tmp/cfghostXXXXXX"; + const char *dir = mkdtemp(tmpl); + assert_true(dir != NULL); + + char linkpath[256]; + xsnprintf(linkpath, sizeof(linkpath), "%s/l", dir); + + char target[CF_BUFSIZE]; + memset(target, 'x', sizeof(target) - 1); + target[sizeof(target) - 1] = '\0'; + if (symlink(target, linkpath) != 0) + { + /* Some platforms (e.g. macOS) refuse targets this long, in which + * case the overflow cannot be reached and there's nothing to test. */ + assert_int_equal(errno, ENAMETOOLONG); + rmdir(dir); + return; + } + + Attributes attr = { 0 }; + attr.link.when_no_file = cfa_delete; + PromiseResult result = PROMISE_RESULT_NOOP; + + assert_false(KillGhostLink(NULL, linkpath, &attr, NULL, &result)); + + unlink(linkpath); + rmdir(dir); +} + +int main() +{ + PRINT_TEST_BANNER(); + + const UnitTest tests[] = { + unit_test(test_expand_links_long_component), + unit_test(test_kill_ghost_link_long_target), + }; + + return run_tests(tests); +}