From 11a7800aee8b4d95baeb13ab15e1dac79d15fdef Mon Sep 17 00:00:00 2001 From: xionglinlin Date: Fri, 12 Jun 2026 17:45:45 +0800 Subject: [PATCH] fix: fix TOCTOU race in wallpaper save function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Add dedicated helper binary (dde-wallpaper-helper) to read files safely with O_NOFOLLOW, eliminating TOCTOU race condition 2. Execute helper via runuser to drop privileges, so kernel enforces the target user's file permissions instead of root bypassing them 3. All file operations (Open/Stat/Read) use the same file descriptor in the helper, leaving zero TOCTOU window 4. Compute MD5 from already-read content instead of re-opening the file 5. Remove unused maxSize constant and dead checkFileReadPermission code Log: Fixed security vulnerability in wallpaper saving functionality. The original TOCTOU fix introduced a privilege-escalation regression (root reading arbitrary user files). This is now resolved by a dedicated helper binary that runs under the target user's identity. Influence: 1. Test saving wallpaper with a regular file to verify functionality 2. Test with a symlink target to confirm symlinks are rejected 3. Test with a file not readable by the target user to confirm access denied 4. Verify MD5 checksum is computed correctly 5. Verify the fix works for both regular and solid color wallpapers 6. Test concurrent operations to ensure race condition is mitigated fix: 修复壁纸保存函数中的TOCTOU竞态漏洞 1. 新增专用 helper 二进制 (dde-wallpaper-helper)，使用 O_NOFOLLOW 安全读取 文件，消除 TOCTOU 竞态条件 2. 通过 runuser 以目标用户身份执行 helper，由内核强制文件权限检查，解决 root 越权读取用户文件的问题 3. helper 内所有文件操作 (Open/Stat/Read) 基于同一文件描述符，TOCTOU 窗口为零 4. 从已读取的内存数据计算 MD5，避免重复打开文件 5. 移除不再使用的 maxSize 常量和 checkFileReadPermission 死代码 Log: 修复壁纸保存功能的安全漏洞。 原 TOCTOU 修复引入了 root 越权读用户文件的权限回退问题， 现通过专用 helper 二进制以目标用户身份运行来解决。 Influence: 1. 使用常规文件测试保存壁纸功能，验证功能正常 2. 使用符号链接目标测试，确认符号链接被拒绝 3. 使用目标用户无读权限的文件测试，确认返回权限拒绝 4. 验证 MD5 校验和计算是否正确 5. 验证修复对常规壁纸和纯色壁纸均有效 6. 测试并发操作以确保竞态条件已被解决 PMS: BUG-364751 Change-Id: I0ed2d3474f0a869e4f61731f53730f6103720785 --- Makefile | 1 + bin/dde-system-daemon/wallpaper.go | 57 ++++++++++++++++-------------- bin/dde-wallpaper-helper/main.go | 55 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 27 deletions(-) create mode 100644 bin/dde-wallpaper-helper/main.go diff --git a/Makefile b/Makefile index 7a7d48bf2..e04d6ccc4 100644 --- a/Makefile +++ b/Makefile @@ -92,6 +92,7 @@ TEST = \ BINARIES = \ dde-session-daemon \ dde-system-daemon \ + dde-wallpaper-helper \ grub2 \ search \ backlight_helper \ diff --git a/bin/dde-system-daemon/wallpaper.go b/bin/dde-system-daemon/wallpaper.go index 545528b1f..68a9e0152 100644 --- a/bin/dde-system-daemon/wallpaper.go +++ b/bin/dde-system-daemon/wallpaper.go @@ -1,10 +1,12 @@ -// SPDX-FileCopyrightText: 2022 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2022 - 2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-or-later package main import ( + "bytes" + "crypto/md5" "errors" "fmt" "os" @@ -23,11 +25,11 @@ import ( ) const maxCount = 20 -const maxSize = 32 * 1024 * 1024 const wallPaperDir = "/var/cache/wallpapers/custom-wallpapers/" const solidWallPaperPath = "/var/cache/wallpapers/custom-solidwallpapers/" const solidPrefix = "solid::" const polkitActionUserAdministration = "org.deepin.dde.accounts.user-administration" +const wallpaperHelperPath = "/usr/lib/deepin-daemon/dde-wallpaper-helper" var wallPaperDirs = []string{ wallPaperDir, @@ -41,7 +43,9 @@ const ( func checkPath(path string, dirs []string) string { for _, dir := range dirs { - if strings.HasPrefix(path, dir) { + // 确保目录路径以分隔符结尾，防止 "user1" 匹配 "user10" + prefix := dir + string(filepath.Separator) + if path == dir || strings.HasPrefix(path, prefix) { return dir } } @@ -210,6 +214,21 @@ func (d *Daemon) checkAuth(sender dbus.Sender) error { return checkAuth(polkitActionUserAdministration, string(sender)) } +// readWallpaperSourceAsUser reads the wallpaper source file with the target +// user's privileges. It executes dde-wallpaper-helper via runuser so the +// kernel enforces the target user's file permissions, and the helper opens +// the file atomically with O_NOFOLLOW to prevent TOCTOU races. +func readWallpaperSourceAsUser(username string, file string) ([]byte, error) { + cmd := exec.Command("runuser", "-u", username, "--", wallpaperHelperPath, file) + var stderr bytes.Buffer + cmd.Stderr = &stderr + src, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("permission denied, %s is not allowed to read this file:%s: %s", username, file, strings.TrimSpace(stderr.String())) + } + return src, nil +} + func (d *Daemon) SaveCustomWallPaper(sender dbus.Sender, username string, file string) (string, *dbus.Error) { var err error var isSolid bool = false @@ -219,18 +238,7 @@ func (d *Daemon) SaveCustomWallPaper(sender dbus.Sender, username string, file s file = strings.TrimPrefix(file, solidPrefix) isSolid = true } - info, err := os.Stat(file) - if err != nil { - logger.Warning(err) - return "", dbusutil.ToError(err) - } - - if info.Size() > maxSize { - err = fmt.Errorf("file size %d > %d", info.Size(), maxSize) - logger.Warning(err) - return "", dbusutil.ToError(err) - } - dirs, _ := GetUserDirs(username) + dirs, err := GetUserDirs(username) if err != nil { logger.Warning(err) @@ -253,7 +261,13 @@ func (d *Daemon) SaveCustomWallPaper(sender dbus.Sender, username string, file s err = fmt.Errorf("%s not allowed to set %s wallpaper", user.Username, username) return "", dbusutil.ToError(err) } - md5sum, _ := dutils.SumFileMd5(file) + + src, err := readWallpaperSourceAsUser(username, file) + if err != nil { + logger.Warning(err) + return "", dbusutil.ToError(err) + } + md5sum := fmt.Sprintf("%x", md5.Sum(src)) var prefix string if isSolid { @@ -279,17 +293,6 @@ func (d *Daemon) SaveCustomWallPaper(sender dbus.Sender, username string, file s if dutils.IsFileExist(destFile) { return destFile, nil } - src, err := exec.Command("runuser", []string{ - "-u", - username, - "--", - "cat", - file, - }...).Output() - if err != nil { - err = fmt.Errorf("permission denied, %s is not allowed to read this file:%s", username, file) - return "", dbusutil.ToError(err) - } err = os.WriteFile(destFile, src, 0644) if err != nil { diff --git a/bin/dde-wallpaper-helper/main.go b/bin/dde-wallpaper-helper/main.go new file mode 100644 index 000000000..bf3295a2a --- /dev/null +++ b/bin/dde-wallpaper-helper/main.go @@ -0,0 +1,55 @@ +// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd. +// +// SPDX-License-Identifier: GPL-3.0-or-later + +package main + +import ( + "fmt" + "io" + "os" + + "golang.org/x/sys/unix" +) + +// maxSize matches the limit enforced by dde-system-daemon's SaveCustomWallPaper. +const maxSize = 32 * 1024 * 1024 + +func main() { + if len(os.Args) != 2 { + _, _ = fmt.Fprintln(os.Stderr, "usage: dde-wallpaper-helper ") + os.Exit(1) + } + + file := os.Args[1] + + // Open with O_NOFOLLOW to atomically reject symlinks — this is the + // core TOCTOU defence. All subsequent operations use the returned fd. + fd, err := unix.Open(file, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + f := os.NewFile(uintptr(fd), file) + defer f.Close() + + info, err := f.Stat() + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + if !info.Mode().IsRegular() { + _, _ = fmt.Fprintf(os.Stderr, "file %s is not a regular file\n", file) + os.Exit(1) + } + if info.Size() > maxSize { + _, _ = fmt.Fprintf(os.Stderr, "file size %d exceeds limit %d\n", info.Size(), maxSize) + os.Exit(1) + } + + _, err = io.Copy(os.Stdout, f) + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +}