[PATCH] util: virFileChownFiles: do not follow symlinks

Daniel P. Berrangé via Devel posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260728165936.398636-1-berrange@redhat.com
src/util/virfile.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
[PATCH] util: virFileChownFiles: do not follow symlinks
Posted by Daniel P. Berrangé via Devel 1 week, 1 day ago
From: HE WEI(ギカク) <skyexpoc@gmail.com>

virFileChownFiles() selected entries with virFileIsRegular() (stat(), follows
symlinks) and changed ownership with chown() (follows symlinks). A component
that owns the target directory at a lower privilege (e.g. the swtpm/tss state
directory) can plant a symlink to an arbitrary regular file and have the root
caller chown that file. Use lstat() to skip non-regular entries and
fchownat(..., AT_SYMLINK_NOFOLLOW) so a symlink final component is never
followed.

Fixes: CVE-2026-63622
Signed-off-by: HE WEI(ギカク) <skyexpoc@gmail.com>
[DB: use g_lstat instead of stat; use lchown instead of
 fchownat for portability; added comment]
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virfile.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index a0c6cb8048..c9d838eeeb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3306,6 +3306,12 @@ int virDirIsEmpty(const char *path,
  *
  * Change ownership of all regular files in a directory.
  *
+ * This will NOT follow any symlinks, to avoid security risks.
+ * It is assumed the process using content under @name will
+ * be unprivileged, thus less trusted than libvirt. If it is
+ * compromised it might attempt to create symlinks in @name to
+ * escalate privileges on a subsequent call to virFileChownFiles.
+ *
  * Returns -1 on error, with error already reported, 0 on success.
  */
 #ifndef WIN32
@@ -3322,13 +3328,19 @@ int virFileChownFiles(const char *name,
 
     while ((direrr = virDirRead(dir, &ent, name)) > 0) {
         g_autofree char *path = NULL;
+        struct stat sb;
 
         path = g_build_filename(name, ent->d_name, NULL);
 
-        if (!virFileIsRegular(path))
+        if (g_lstat(path, &sb) < 0) {
+            virReportSystemError(errno, _("cannot stat '%1$s'"), path);
+            return -1;
+        }
+
+        if (!S_ISREG(sb.st_mode))
             continue;
 
-        if (chown(path, uid, gid) < 0) {
+        if (lchown(path, uid, gid) < 0) {
             virReportSystemError(errno,
                                  _("cannot chown '%1$s' to (%2$u, %3$u)"),
                                  ent->d_name, (unsigned int) uid,
-- 
2.55.0

Re: [PATCH] util: virFileChownFiles: do not follow symlinks
Posted by Ján Tomko via Devel 6 days, 13 hours ago
On a Tuesday in 2026, Daniel P. Berrangé via Devel wrote:
>From: HE WEI(ギカク) <skyexpoc@gmail.com>
>
>virFileChownFiles() selected entries with virFileIsRegular() (stat(), follows
>symlinks) and changed ownership with chown() (follows symlinks). A component
>that owns the target directory at a lower privilege (e.g. the swtpm/tss state
>directory) can plant a symlink to an arbitrary regular file and have the root
>caller chown that file. Use lstat() to skip non-regular entries and
>fchownat(..., AT_SYMLINK_NOFOLLOW) so a symlink final component is never
>followed.
>
>Fixes: CVE-2026-63622
>Signed-off-by: HE WEI(ギカク) <skyexpoc@gmail.com>
>[DB: use g_lstat instead of stat; use lchown instead of
> fchownat for portability; added comment]
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> src/util/virfile.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano