Fixed stack overflows in symlink path handling (3.24.x)

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;
+                }
             }
         }
     }

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')
         {

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
 

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 <test.h>
+
+#include <cf3.defs.h>
+#include <files_links.h>                             /* ExpandLinks, KillGhostLink */
+#include <attributes.h>                                            /* Attributes */
+#include <unistd.h>                                          /* symlink, unlink */
+#include <errno.h>                                                 /* 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);
+}