fix: filter clipboard data for plugin Wayland clients#1561
fix: filter clipboard data for plugin Wayland clients#1561deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
The clipboard synchronization to plugin Wayland clients was passing through all MIME data formats from the host system, which could include sensitive or unsupported data types. This change filters the clipboard data to only allow "text/plain" and "text/html" formats before forwarding to the compositor. A lightweight QMimeData object is created with only the allowed formats to prevent exposing unnecessary data. This improves security and compatibility with plugin clients. fix: 为插件 Wayland 客户端过滤剪贴板数据 之前向插件 Wayland 客户端同步剪贴板数据时,会传递宿主系统的所有 MIME 数 据格式,这可能包含敏感或不支持的数据类型。此更改在将数据转发到合成器之 前,将剪贴板数据过滤为仅允许 "text/plain" 和 "text/html" 格式。创建一个 仅包含允许格式的轻量级 QMimeData 对象,以防止暴露不必要的数据。这提高了 安全性和与插件客户端的兼容性。 PMS: BUG-357249
Reviewer's GuideFilters clipboard data before synchronizing it to plugin Wayland clients by allowing only text/plain and text/html MIME types via a lightweight QMimeData, and refactors clipboard sync into a reusable helper lambda used both on change and at initialization. Sequence diagram for filtered clipboard sync to plugin Wayland clientssequenceDiagram
participant HostApp
participant QClipboard
participant PluginManager
participant syncClipboardData
participant Compositor
HostApp->>PluginManager: initialize()
PluginManager->>Compositor: setRetainedSelectionEnabled(true)
PluginManager->>QClipboard: mimeData(Clipboard)
QClipboard-->>PluginManager: mimeData
PluginManager->>syncClipboardData: mimeData
syncClipboardData->>syncClipboardData: iterate formats
syncClipboardData->>syncClipboardData: copy only text_plain and text_html
alt has allowed formats
syncClipboardData->>Compositor: overrideSelection(lightweightData)
end
QClipboard-->>PluginManager: changed(Clipboard)
PluginManager->>QClipboard: mimeData(Clipboard)
QClipboard-->>PluginManager: mimeData
PluginManager->>syncClipboardData: mimeData
syncClipboardData->>syncClipboardData: iterate formats
syncClipboardData->>syncClipboardData: copy only text_plain and text_html
alt has allowed formats
syncClipboardData->>Compositor: overrideSelection(lightweightData)
end
Flow diagram for clipboard MIME type filtering before compositor syncflowchart TD
A_start["Clipboard data available"] --> B_checkMime["Get QMimeData from QClipboard"]
B_checkMime --> C_nullCheck{mimeData is null?}
C_nullCheck -- Yes --> Z_end["Return without syncing"]
C_nullCheck -- No --> D_initLightweight["Create lightweight QMimeData"]
D_initLightweight --> E_initFlag["Set hasData to false"]
E_initFlag --> F_loopFormats["For each format in mimeData.formats"]
F_loopFormats --> G_allowedCheck{Format is text/plain or text/html?}
G_allowedCheck -- No --> F_loopFormats
G_allowedCheck -- Yes --> H_copyData["Copy data for format into lightweight QMimeData"]
H_copyData --> I_setFlag["Set hasData to true"]
I_setFlag --> F_loopFormats
F_loopFormats --> J_afterLoop["After iterating all formats"]
J_afterLoop --> K_hasDataCheck{hasData is true?}
K_hasDataCheck -- No --> Z_end
K_hasDataCheck -- Yes --> L_override["Call compositor.overrideSelection with lightweight QMimeData"]
L_override --> Z_end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Passing
&lightweightDatatocompositor->overrideSelection()risks a dangling pointer if the compositor stores theQMimeData*beyond the call; consider ensuringoverrideSelectioncopies the data or allocating a heap-ownedQMimeDatawith a clear ownership model. - Filtering strictly on
"text/plain"and"text/html"MIME types may miss common variants like"text/plain;charset=utf-8"; consider usinghasFormator a more flexible match (e.g., prefix orstartsWith("text/plain")) if those formats should also be supported.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Passing `&lightweightData` to `compositor->overrideSelection()` risks a dangling pointer if the compositor stores the `QMimeData*` beyond the call; consider ensuring `overrideSelection` copies the data or allocating a heap-owned `QMimeData` with a clear ownership model.
- Filtering strictly on `"text/plain"` and `"text/html"` MIME types may miss common variants like `"text/plain;charset=utf-8"`; consider using `hasFormat` or a more flexible match (e.g., prefix or `startsWith("text/plain")`) if those formats should also be supported.
## Individual Comments
### Comment 1
<location path="panels/dock/pluginmanagerextension.cpp" line_range="523-532" />
<code_context>
- if (mode == QClipboard::Clipboard) {
- if (const QMimeData *mimeData = QGuiApplication::clipboard()->mimeData(mode)) {
- compositor->overrideSelection(mimeData);
+ auto syncClipboardData = [compositor](const QMimeData *mimeData) {
+ if (!mimeData) return;
+ QMimeData lightweightData;
+ static QStringList allowedFormats = {"text/plain", "text/html"};
+ bool hasData = false;
+
+ for (const QString &format : mimeData->formats()) {
+ if (allowedFormats.contains(format)) {
+ lightweightData.setData(format, mimeData->data(format));
+ hasData = true;
}
}
+
+ if (hasData) {
+ compositor->overrideSelection(&lightweightData);
+ }
+ };
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid passing a pointer to a stack-allocated QMimeData into overrideSelection.
`lightweightData` is a stack object whose address is passed to `overrideSelection`. This is only safe if `overrideSelection` is strictly synchronous and never stores the pointer. To avoid lifetime issues, either change `overrideSelection` to take `QMimeData` by value/const reference and copy as needed, or allocate `lightweightData` on the heap with well-defined ownership.
</issue_to_address>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这段代码修改主要优化了剪贴板数据同步的逻辑,引入了数据过滤机制。以下是对该diff的详细审查和改进建议: 1. 语法逻辑审查优点:
潜在问题:
2. 代码质量审查优点:
改进建议:
3. 代码性能审查优点:
潜在问题:
4. 代码安全审查优点:
改进建议:
5. 其他建议
改进后的代码示例void PluginManager::initialize()
{
compositor->setRetainedSelectionEnabled(true);
if (auto *clipboard = QGuiApplication::clipboard()) {
// 使用const和更明确的命名
static const QStringList allowedFormats = {"text/plain", "text/html"};
auto syncClipboardData = [compositor](const QMimeData *mimeData) {
if (!mimeData) return;
// 提前检查是否有需要的数据
if (!mimeData->hasText() && !mimeData->hasHtml()) {
return;
}
QMimeData lightweightData;
bool hasData = false;
// 遍历允许的格式
for (const QString &format : allowedFormats) {
if (mimeData->hasFormat(format)) {
lightweightData.setData(format, mimeData->data(format));
hasData = true;
}
}
if (hasData) {
compositor->overrideSelection(&lightweightData);
}
};
QObject::connect(clipboard, &QClipboard::changed, this, [clipboard, syncClipboardData](QClipboard::Mode mode) {
if (mode == QClipboard::Clipboard) {
syncClipboardData(clipboard->mimeData(mode));
}
});
syncClipboardData(clipboard->mimeData(QClipboard::Clipboard));
}
}总结这段代码修改在安全性和性能上都有所提升,但可以通过进一步优化代码结构和添加安全检查来提高代码质量。建议确认 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, 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 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
The clipboard synchronization to plugin Wayland clients was passing through all MIME data formats from the host system, which could include sensitive or unsupported data types. This change filters the clipboard data to only allow "text/plain" and "text/html" formats before forwarding to the compositor. A lightweight QMimeData object is created with only the allowed formats to prevent exposing unnecessary data. This improves security and compatibility with plugin clients.
fix: 为插件 Wayland 客户端过滤剪贴板数据
之前向插件 Wayland 客户端同步剪贴板数据时,会传递宿主系统的所有 MIME 数
据格式,这可能包含敏感或不支持的数据类型。此更改在将数据转发到合成器之
前,将剪贴板数据过滤为仅允许 "text/plain" 和 "text/html" 格式。创建一个
仅包含允许格式的轻量级 QMimeData 对象,以防止暴露不必要的数据。这提高了
安全性和与插件客户端的兼容性。
PMS: BUG-357249
Summary by Sourcery
Bug Fixes: