fix: narrow path traversal check pattern in DEnumerator buildUrl - #380
Conversation
将路径遍历检查从 contains('/') || contains('\\') || == "." || == ".."
收窄为 contains("../") || contains("..\\") || startsWith(".."),
与 release/eagle、release/snipe 分支保持一致,只拦截实际的路径遍历尝试。
PMS: BUG-372733
Bug: https://pms.uniontech.com/bug-view-372733.html
There was a problem hiding this comment.
Sorry @Johnson-zs, 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: Johnson-zs 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideNarrows the path traversal check in DEnumeratorPrivate::buildUrl to only catch real directory traversal attempts while keeping the restriction scoped to file:// (or empty) schemes. Flow diagram for updated path traversal check in buildUrlflowchart TD
A[buildUrl url fileName] --> B[get scheme]
B --> C{scheme is empty or file}
C -- no --> D[proceed to build QUrl]
C -- yes --> E{fileNameBa contains ../ or ..\\ or startsWith ..}
E -- yes --> F[return empty QUrl]
E -- no --> D[proceed to build QUrl]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review★ 总体评分:40分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 diff --git a/src/dfm-io/dfm-io/denumerator.cpp b/src/dfm-io/dfm-io/denumerator.cpp
index c5aaa639..12345678 100644
--- a/src/dfm-io/dfm-io/denumerator.cpp
+++ b/src/dfm-io/dfm-io/denumerator.cpp
@@ -463,7 +463,10 @@ QUrl DEnumeratorPrivate::buildUrl(const QUrl &url, const char *fileName)
// 这是合法的 trash 文件名而非恶意路径,故不应对其做路径遍历拦截。
const QString scheme = url.scheme();
if (scheme.isEmpty() || scheme == QLatin1String("file")) {
- if (fileNameBa.contains('/') || fileNameBa.contains('\\') || fileNameBa == "." || fileNameBa == "..") {
+ if (fileNameBa.startsWith('/') || fileNameBa.startsWith('\\') ||
+ fileNameBa.contains("../") || fileNameBa.contains("..\\") ||
+ fileNameBa.endsWith("/..") || fileNameBa.endsWith("\\..") ||
+ fileNameBa == "..") {
return QUrl();
}
} |
|
@Johnson-zs: 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. |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
补充修复
PR #377 已合入 master,将路径遍历检查限制为仅对
file:///scheme 生效(修复了 gio trash 回收站问题)。本 PR 将路径遍历检查模式从
contains('/') || contains('\\') || == "." || == ".."收窄为contains("../") || contains("..\\") || startsWith(".."),与 release/eagle、release/snipe 分支保持一致,只拦截实际的路径遍历尝试。PMS: BUG-372733
Summary by Sourcery
Bug Fixes: