fix(security): harden ops log export via fd-based transfer and symlin… - #485
Closed
GongHeng2017 wants to merge 1 commit into
Closed
fix(security): harden ops log export via fd-based transfer and symlin…#485GongHeng2017 wants to merge 1 commit into
GongHeng2017 wants to merge 1 commit into
Conversation
![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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017 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 |
|
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\";",
" 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);"
]
}
}
} |
…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
GongHeng2017
force-pushed
the
202607072144-master-fix
branch
from
July 7, 2026 13:49
3cd822b to
9b8846d
Compare
|
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\";",
" 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★ 总体评分：60分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 修复 LogViewerService::exportOpsLog 中的栈溢出与进程阻塞漏洞
bool LogViewerService::exportOpsLog(const QDBusUnixFileDescriptor &fd)
{
// ... 前置鉴权与临时目录创建逻辑保持不变 ...
QProcess zipProc;
zipProc.setWorkingDirectory(logCollectDir);
zipProc.start(QStringLiteral("zip"), QStringList() << QStringLiteral("-r")
<< tmpZipPath << QStringLiteral("."));
// 修复安全漏洞2：增加超时限制，防止进程无限阻塞导致服务挂起
static constexpr int kZipTimeoutMs = 300000;
if (!zipProc.waitForFinished(kZipTimeoutMs) || zipProc.exitCode() != 0) {
qCWarning(logService) << "exportOpsLog: zip failed or timed out, exitCode:" << zipProc.exitCode()
<< "stderr:" << zipProc.readAllStandardError();
zipProc.kill();
QFile::remove(tmpZipPath);
removeOpsTempDirByPathInternal(opsDir);
return false;
}
bool writeOk = false;
{
QFile zipIn(tmpZipPath);
if (!zipIn.open(QIODevice::ReadOnly)) {
qCWarning(logService) << "exportOpsLog: failed to open temp zip for reading:" << tmpZipPath;
} else {
QFile fdOut;
if (!fdOut.open(fdi, QIODevice::WriteOnly)) {
qCWarning(logService) << "exportOpsLog: failed to open caller fd for writing";
} else {
constexpr qint64 bufSize = 1 << 20; // 1 MiB
// 修复安全漏洞1：将栈分配改为堆分配，避免栈溢出导致守护进程崩溃
QByteArray buffer(bufSize, Qt::Uninitialized);
char *buf = buffer.data();
qint64 n = 0;
bool error = false;
while ((n = zipIn.read(buf, bufSize)) > 0) {
qint64 written = 0;
while (written < n) {
qint64 w = fdOut.write(buf + written, n - written);
if (w < 0) {
error = true;
break;
}
written += w;
}
if (error)
break;
}
if (error || n < 0) {
qCWarning(logService) << "exportOpsLog: write to caller fd failed";
} else {
fdOut.flush();
writeOk = true;
}
fdOut.close();
}
zipIn.close();
}
}
QFile::remove(tmpZipPath);
removeOpsTempDirByPathInternal(opsDir);
if (!writeOk) {
qCWarning(logService) << "exportOpsLog: aborted, failed to write zip to caller fd";
return false;
}
qCDebug(logService) << "exportOpsLog: ops logs zipped and written to caller fd successfully";
return true;
} |
|
@GongHeng2017: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
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
2 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