fix(bluetooth): prevent path traversal in OBEX file receive#1097
Merged
mhduiy merged 1 commit intolinuxdeepin:masterfrom May 7, 2026
Merged
fix(bluetooth): prevent path traversal in OBEX file receive#1097mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy merged 1 commit intolinuxdeepin:masterfrom
Conversation
1. Add validateFilename() to reject filenames with path separators 2. Block relative/absolute path traversal attacks (../, /etc/passwd) 3. Prevent symlink attacks by checking destination before move 4. Add comprehensive unit tests for filename validation Log: Fix path traversal vulnerability in Bluetooth OBEX file receive fix(bluetooth): 修复蓝牙 OBEX 文件接收路径遍历漏洞 1. 添加 validateFilename() 拒绝包含路径分隔符的文件名 2. 阻止相对/绝对路径遍历攻击（../、/etc/passwd） 3. 移动文件前检查目标是否为符号链接，防止符号链接攻击 4. 添加文件名验证的完整单元测试 Log: 修复蓝牙 OBEX 文件接收路径遍历漏洞
Reviewer's GuideAdds server-side filename validation and symlink protections to the Bluetooth OBEX file receive path to block path traversal and symlink-based attacks, along with unit tests to cover the new validation logic. Sequence diagram for secure OBEX file receive and movesequenceDiagram
actor RemoteDevice
participant obexAgent
participant validateFilenameUtil
participant TempStorage
participant FileMover
participant Filesystem
RemoteDevice->>obexAgent: AuthorizePush(transferPath)
obexAgent->>TempStorage: Get transfer properties
TempStorage-->>obexAgent: oriFilename
obexAgent->>validateFilenameUtil: validateFilename(oriFilename)
alt invalid filename
validateFilenameUtil-->>obexAgent: error
obexAgent-->>RemoteDevice: dbus error invalid filename
else valid filename
validateFilenameUtil-->>obexAgent: nil
obexAgent->>TempStorage: randFileName(oriFilename)
TempStorage-->>obexAgent: tempFileName
obexAgent-->>RemoteDevice: tempFileName
Note over RemoteDevice,obexAgent: file transfer completes
obexAgent->>FileMover: moveTempFile(srcTempPath, destPath)
FileMover->>Filesystem: Lstat(destPath)
alt dest is symlink
Filesystem-->>FileMover: FileInfo symlink
FileMover-->>obexAgent: ""
else dest not symlink
Filesystem-->>FileMover: FileInfo regular or error
FileMover->>Filesystem: rename(srcTempPath, destPath)
Filesystem-->>FileMover: ok
FileMover-->>obexAgent: destPath
end
end
Class diagram for updated OBEX agent filename validation and file move logicclassDiagram
class obexAgent {
+AuthorizePush(transferPath dbus.ObjectPath) string
+receiveProgress(transfer transferObj)
}
class validateFilenameUtil {
+validateFilename(name string) error
}
class FileMover {
+moveTempFile(src string, dest string) string
}
obexAgent ..> validateFilenameUtil : uses
obexAgent ..> FileMover : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The symlink pre-check in
moveTempFileintroduces a TOCTOU race (an attacker can swap in a symlink betweenos.Lstatand the subsequent operations); consider using an atomic-safe pattern (e.g., creating/opening the destination withO_EXCLor similar) instead of a separate pre-check. validateFilenamecurrently rejects any name containing/or\and then also checksfilepath.Base(name) != name, which is redundant given the earlier checks; you can simplify by removing thefilepath.Basecomparison or explain why both are needed for a specific edge case.- The two test functions around
validateFilename(TestValidateFilenameandTestValidateFilenameSecurity) have overlapping coverage; consider consolidating them into a single table-driven test to reduce duplication and make it easier to add/modify cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The symlink pre-check in `moveTempFile` introduces a TOCTOU race (an attacker can swap in a symlink between `os.Lstat` and the subsequent operations); consider using an atomic-safe pattern (e.g., creating/opening the destination with `O_EXCL` or similar) instead of a separate pre-check.
- `validateFilename` currently rejects any name containing `/` or `\` and then also checks `filepath.Base(name) != name`, which is redundant given the earlier checks; you can simplify by removing the `filepath.Base` comparison or explain why both are needed for a specific edge case.
- The two test functions around `validateFilename` (`TestValidateFilename` and `TestValidateFilenameSecurity`) have overlapping coverage; consider consolidating them into a single table-driven test to reduce duplication and make it easier to add/modify cases.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review这段代码主要是为了增强蓝牙 OBEX 文件传输的安全性，防止路径遍历攻击和符号链接攻击。以下是对代码的详细审查和改进建议： 1. 语法逻辑审查优点：
改进建议：
2. 代码质量审查优点：
改进建议：
3. 代码性能审查优点：
改进建议：
4. 代码安全审查优点：
改进建议：
改进后的代码示例// validateFilename validates that the filename is safe and does not contain
// path traversal sequences or control characters. Only pure filenames are accepted.
//
// Security: Prevents path traversal attacks where a malicious remote device
// sends filenames like "../.config/autostart/evil.desktop" to write files
// outside the intended download directory.
func validateFilename(name string) error {
// Reject empty filename
if name == "" {
return errors.New("empty filename")
}
// Reject special directory names
if name == "." || name == ".." {
return errors.New("invalid filename: . or ..")
}
// Reject path separators - filename must not contain '/' or '\'
// This prevents absolute paths and relative path traversal
if strings.ContainsAny(name, "/\\") {
return errors.New("path separator not allowed in filename")
}
// Reject control characters
for _, r := range name {
if r < 32 || r == 127 {
return errors.New("control characters not allowed in filename")
}
}
// Ensure the name is a pure filename, not a path
// filepath.Base("../foo") returns "foo", which != "../foo"
if filepath.Base(name) != name {
return errors.New("not a pure filename")
}
// Limit filename length to prevent potential issues
if len(name) > 255 {
return errors.New("filename too long")
}
return nil
}
func moveTempFile(src, dest string) string {
// Security: Check if destination is a symlink or directory to prevent attacks.
// A malicious local user could create a symlink in the download directory
// pointing to a sensitive file (e.g., ~/.bashrc, ~/.config/autostart/*.desktop).
// Use os.Lstat to detect symlinks without following them.
if fi, err := os.Lstat(dest); err == nil {
if fi.Mode()&os.ModeSymlink != 0 {
logger.Error("refusing to overwrite symlink:", dest)
return ""
}
if fi.IsDir() {
logger.Error("refusing to overwrite directory:", dest)
return ""
}
}
count := 0
suffix := filepath.Ext(dest)
fileName := strings.TrimSuffix(dest, suffix)
// ... rest of the function
}总结这段代码在安全性方面做得很好，主要改进点在于：
这些改进将使代码更加健壮和安全，同时保持良好的可读性和可维护性。 |
fly602
approved these changes
May 7, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: Fix path traversal vulnerability in Bluetooth OBEX file receive
fix(bluetooth): 修复蓝牙 OBEX 文件接收路径遍历漏洞
Log: 修复蓝牙 OBEX 文件接收路径遍历漏洞
Summary by Sourcery
Harden Bluetooth OBEX file reception against path traversal and symlink-based attacks.
Bug Fixes:
Tests: