fix(security): harden ops log export via fd-based transfer and symlin… - #486
Merged
deepin-bot[bot] merged 1 commit intoJul 8, 2026
Merged
Conversation
…k-safe collection Refactor the exportOpsLog flow to eliminate path exposure and symlink traversal risks: - Change exportOpsLog DBus API from returning a /tmp path to accepting a caller-provided file descriptor and streaming the zipped logs back over it (bool return), so the backend no longer exposes /var/log paths to callers and the frontend no longer needs a separate cleanup call. - Move user-permission log collection (per-user ~/.cache logs) out of the root backend into Utils::exportUserPermission* helpers in the application, while the backend OpsLogExport now only collects root-owned /var/log logs. - Create the backend temp dir under /var/log instead of /tmp, and clean it up with a TOCTOU-safe, fd-relative recursive removal (openat/fstatat/unlinkat with O_NOFOLLOW) instead of path-based deletion. - Skip symlink sources during copy and switch directory copy from "cp -rf" to "cp -rP" to preserve rather than follow symlinks. - Drop setDirectoryPermissionsSafe chown loop, no longer needed since ownership is not handed back to a non-root caller. - Add ReadWritePaths=/var/log to the systemd unit so the backend can create its temp export dir under the ProtectSystem=strict tree. - Update the D-Bus introspection XML for the new exportOpsLog signature. Log: fix issue Bug: https://pms.uniontech.com/bug-view-368003.html
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @GongHeng2017, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning
详情 {
"export": {
"application/dbusproxy/dldbushandler.cpp": {
"a": [
"QString DLDBusHandler::exportOpsLog()",
" QDBusPendingReply<QString> reply = m_dbus->exportOpsLog();",
" qCWarning(logApp) << \"call dbus iterface 'exportOpsLog()' failed. error info:\" << reply.error().message();"
],
"b": [
"bool DLDBusHandler::exportOpsLog(const QString &zipFilePath)",
" qCritical() << \"exportOpsLog: failed to open zip file for writing:\" << zipFilePath",
" qCritical() << \"exportOpsLog: invalid file descriptor for:\" << zipFilePath;",
" QDBusPendingReply<bool> reply = m_dbus->exportOpsLog(dbusFd);",
" qCritical() << \"call dbus interface 'exportOpsLog' failed. error info:\" << reply.error().message();"
]
},
"application/dbusproxy/dldbushandler.h": {
"a": [
" QString exportOpsLog();"
],
"b": [
" bool exportOpsLog(const QString &zipFilePath);"
]
},
"application/dbusproxy/dldbusinterface.h": {
"a": [
" inline QDBusPendingReply<QString> exportOpsLog()"
],
"b": [
" inline QDBusPendingReply<bool> exportOpsLog(const QDBusUnixFileDescriptor &fd)"
]
},
"application/logallexportthread.cpp": {
"a": [
" QString opsLogPath = DLDBusHandler::instance(this)->exportOpsLog();",
" Utils::exportSomeOpsLogs(opsLogPath, userHomePath);"
],
"b": [
" emit exportFinsh(false);",
" bool opsOk = DLDBusHandler::instance(this)->exportOpsLog(opsZipPath);",
" qCCritical(logApp) << \"exportOpsLog failed or zip not produced\";",
" emit exportFinsh(false);",
" Utils::exportUserPermissionOpsLogs(tmpOpsDirPath, userHomePath);"
]
},
"application/opslogpaths.h": {
"b": [
"// \u524d\u7aef\uff08application/utils.cpp \u7684 exportUserPermissionOpsLogs \u7cfb\u5217\uff09\u4e0e",
"// root \u670d\u52a1\uff08logViewerService/opslogexport.cpp \u7684 createOpsLogDirStruct\uff09\u5fc5\u987b"
]
},
"application/utils.cpp": {
"a": [
"void Utils::exportSomeOpsLogs(const QString &outDir, const QString &userHomeDir)"
],
"b": [
"void Utils::exportUserPermissionAppLogs(const QString &outDir, const QString &userHomeDir)",
"void Utils::exportUserPermissionSystemLogs(const QString &outDir, const QString &userHomeDir)",
"void Utils::exportUserPermissionKernelLogs(const QString &outDir)",
"void Utils::exportUserPermissionDDELogs(const QString &outDir, const QString &userHomeDir)",
"void Utils::exportUserPermissionOpsLogs(const QString &outDir, const QString &userHomeDir)",
" Utils::exportUserPermissionAppLogs(outDir, userHomeDir);",
" Utils::exportUserPermissionSystemLogs(outDir, userHomeDir);",
" Utils::exportUserPermissionKernelLogs(outDir);",
" Utils::exportUserPermissionDDELogs(outDir, userHomeDir);"
]
},
"application/utils.h": {
"a": [
" static void exportSomeOpsLogs(const QString &outDir, const QString &userHomeDir);"
],
"b": [
" static void exportUserPermissionAppLogs(const QString &outDir, const QString &userHomeDir);",
" static void exportUserPermissionSystemLogs(const QString &outDir, const QString &userHomeDir);",
" static void exportUserPermissionKernelLogs(const QString &outDir);",
" static void exportUserPermissionDDELogs(const QString &outDir, const QString &userHomeDir);",
" static void exportUserPermissionOpsLogs(const QString &outDir, const QString &userHomeDir);"
]
},
"logViewerService/assets/data/com.deepin.logviewer.xml": {
"b": [
" <method name=\"exportOpsLog\">"
]
},
"logViewerService/assets/data/deepin-log-viewer-daemon.service": {
"b": [
"# exportOpsLog \u9700\u8981\u5728 /var/log \u4e0b\u521b\u5efa\u4e34\u65f6\u5bfc\u51fa\u76ee\u5f55\uff0c\u987b\u663e\u5f0f\u653e\u5f00\u5199\u6743\u9650"
]
},
"logViewerService/logviewerservice.cpp": {
"a": [
"QString LogViewerService::exportOpsLog()"
],
"b": [
"// \u57fa\u4e8e fd \u7684 TOCTOU \u5b89\u5168\u65b9\u5f0f\u5220\u9664 exportOpsLog \u4ea7\u751f\u7684 /var/log \u4e34\u65f6\u76ee\u5f55\uff08opsDir\uff09\u3002",
"// \u7531 exportOpsLog \u5728\u5199\u5165 fd \u5b8c\u6210\u540e\u81ea\u52a8\u8c03\u7528\uff0c\u786e\u4fdd /var/log \u4e0b\u4e0d\u6b8b\u7559\u542b\u7cfb\u7edf\u65e5\u5fd7\u7684\u76ee\u5f55\u3002",
"bool LogViewerService::exportOpsLog(const QDBusUnixFileDescriptor &fd)",
" qCWarning(logService) << \"exportOpsLog: invalid file descriptor from caller\";",
" qCWarning(logService) << \"exportOpsLog: failed to create temporary dir under /var/log:\" << tmpOpsDir.errorString();",
" qCWarning(logService) << \"exportOpsLog: failed to create log collect dir:\" << logCollectDir;",
" qCWarning(logService) << \"exportOpsLog: zip failed, exitCode:\" << zipProc.exitCode()",
" qCWarning(logService) << \"exportOpsLog: failed to open temp zip for reading:\" << tmpZipPath;",
" qCWarning(logService) << \"exportOpsLog: failed to open caller fd for writing\";",
" // \u4e0e\u672c\u6587\u4ef6 exportLog() \u4e2d 1 MiB \u7f13\u51b2\u533a\u7684\u5904\u7406\u65b9\u5f0f\u4fdd\u6301\u4e00\u81f4\u3002",
" qCWarning(logService) << \"exportOpsLog: write to caller fd failed\";",
" qCWarning(logService) << \"exportOpsLog: aborted, failed to write zip to caller fd\";",
" qCDebug(logService) << \"exportOpsLog: ops logs zipped and written to caller fd successfully\";"
]
},
"logViewerService/logviewerservice.h": {
"a": [
" Q_SCRIPTABLE QString exportOpsLog();"
],
"b": [
" Q_SCRIPTABLE bool exportOpsLog(const QDBusUnixFileDescriptor &fd);",
" // \u57fa\u4e8e fd \u7684 TOCTOU \u5b89\u5168\u65b9\u5f0f\u5220\u9664 exportOpsLog \u4ea7\u751f\u7684 /var/log \u4e34\u65f6\u76ee\u5f55\uff08opsDir\uff09\u3002",
" // \u7531 exportOpsLog \u5199\u5165 fd \u5b8c\u6210\u540e\u81ea\u52a8\u8c03\u7528\u56de\u6536\uff0c\u6210\u529f\u8fd4\u56de true\u3002"
]
},
"logViewerService/opslogexport.cpp": {
"a": [
" qWarning() << \"Failed to chown export path:\" << path << \"error:\" << strerror(errno);"
]
}
}
} |
deepin pr auto review★ 总体评分：95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // application/logallexportthread.cpp
// 增加对 unzip 解压结果的校验，防止解压失败时将空目录打入最终压缩包
QByteArray unzipOutput = Utils::executeCmd("unzip", QStringList() << "-n" << opsZipPath << "-d" << tmpOpsDirPath);
if (unzipOutput.isEmpty()) {
qCCritical(logApp) << "unzip failed or produced no output for:" << opsZipPath;
zipClose(m_zipFile, nullptr);
QFile::remove(m_outfile);
emit exportFinsh(false);
return;
}
// 及时清理 opsZipPath 压缩包，避免将该压缩包也导出给用户
QFile::remove(opsZipPath); |
lzwind
approved these changes
Jul 8, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, lzwind 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
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.
…k-safe collection
Refactor the exportOpsLog flow to eliminate path exposure and symlink traversal risks:
Log: fix issue
Bug: https://pms.uniontech.com/bug-view-368003.html