fix(WPF): deny WPF Behaviors namespaces in untrusted homepage XAML#3049
Open
lhx077 wants to merge 1 commit into
Open
fix(WPF): deny WPF Behaviors namespaces in untrusted homepage XAML#3049lhx077 wants to merge 1 commit into
lhx077 wants to merge 1 commit into
Conversation
审阅者指南(在小型 PR 上折叠)审阅者指南在 更新后的
|
| 变更 | 详情 | 文件 |
|---|---|---|
添加预解析拒绝列表,在不可信的主页 XAML 中阻止不安全的 WPF Behaviors 命名空间,一旦遇到则抛出 UnauthorizedAccessException。 |
|
Plain Craft Launcher 2/Modules/Base/ModBase.cs |
提示与命令
与 Sourcery 交互
- 触发新的代码审阅: 在 pull request 中评论
@sourcery-ai review。 - 继续讨论: 直接回复 Sourcery 的审阅评论。
- 从审阅评论生成 GitHub issue: 在某条审阅评论下回复,要求 Sourcery 基于该评论创建 issue。你也可以直接回复
@sourcery-ai issue来从该评论创建 issue。 - 生成 pull request 标题: 在 pull request 标题中任意位置写上
@sourcery-ai,即可随时生成标题。你也可以在 pull request 中评论@sourcery-ai title来(重新)生成标题。 - 生成 pull request 摘要: 在 pull request 正文的任意位置写上
@sourcery-ai summary,即可在你想要的位置生成 PR 摘要。你也可以在 pull request 中评论@sourcery-ai summary来在任何时候(重新)生成摘要。 - 生成审阅者指南: 在 pull request 中评论
@sourcery-ai guide,即可在任何时候(重新)生成审阅者指南。 - 批量解决所有 Sourcery 评论: 在 pull request 中评论
@sourcery-ai resolve来将所有 Sourcery 评论标记为已解决。如果你已经处理完所有评论且不想再看到它们,这会很有用。 - 忽略所有 Sourcery 审阅: 在 pull request 中评论
@sourcery-ai dismiss来忽略所有现有的 Sourcery 审阅。尤其适用于你想从头开始一次新的审阅——别忘了评论@sourcery-ai review来触发新的审阅!
自定义你的体验
访问你的 控制面板 来:
- 启用或禁用审阅功能,如 Sourcery 生成的 pull request 摘要、审阅者指南等。
- 更改审阅使用的语言。
- 添加、删除或编辑自定义审阅指令。
- 调整其他审阅设置。
获取帮助
Original review guide in English
Reviewer's guide (collapsed on small PRs)
Reviewer's Guide
Adds a pre-parse XAML namespace denylist in ModBase.GetObjectFromXML to block unsafe WPF Behaviors namespaces in untrusted homepage XAML while preserving existing compatibility rewrites and XamlXmlReader-based checks.
Sequence diagram for updated ModBase.GetObjectFromXML XAML security checks
sequenceDiagram
participant HomepageLoader
participant ModBase
participant XamlReader
HomepageLoader->>ModBase: GetObjectFromXML(str)
ModBase->>ModBase: Apply compatibility Replace calls
ModBase->>ModBase: Scan for blockedXamlNamespace
alt blocked namespace found
ModBase-->>HomepageLoader: throw UnauthorizedAccessException
else no blocked namespace
ModBase->>XamlReader: Load(stream)
XamlReader-->>ModBase: object
ModBase-->>HomepageLoader: object
end
File-Level Changes
| Change | Details | Files |
|---|---|---|
| Add a pre-parse denylist that blocks unsafe WPF Behaviors namespaces in untrusted homepage XAML and throws UnauthorizedAccessException when encountered. |
|
Plain Craft Launcher 2/Modules/Base/ModBase.cs |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
There was a problem hiding this comment.
Hey - 我在这里给出一些高层面的反馈:
string.Contains(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase)这个重载在较旧的 .NET Framework 目标中不可用;为避免潜在的编译问题,建议改用IndexOf(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase) >= 0。- 被阻止的 XAML 命名空间列表在每次调用
GetObjectFromXML时都会被重新分配;这是一个热点路径,建议将该列表移动到一个静态 readonly 字段中,以避免重复的数组分配。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `string.Contains(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase)` overload is not available on older .NET Framework targets; consider using `IndexOf(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase) >= 0` instead to avoid potential compilation issues.
- The blocked XAML namespace list is reallocated on every `GetObjectFromXML` call; consider moving it to a static readonly field to avoid repeated array allocations in this hot path.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈来改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The
string.Contains(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase)overload is not available on older .NET Framework targets; consider usingIndexOf(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase) >= 0instead to avoid potential compilation issues. - The blocked XAML namespace list is reallocated on every
GetObjectFromXMLcall; consider moving it to a static readonly field to avoid repeated array allocations in this hot path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `string.Contains(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase)` overload is not available on older .NET Framework targets; consider using `IndexOf(blockedXamlNamespace, StringComparison.OrdinalIgnoreCase) >= 0` instead to avoid potential compilation issues.
- The blocked XAML namespace list is reallocated on every `GetObjectFromXML` call; consider moving it to a static readonly field to avoid repeated array allocations in this hot path.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
whitecat346
requested changes
Jun 6, 2026
| Replace("Property=\"EventData\"", "Property=\"local:CustomEventService.EventData\""); | ||
|
|
||
| // 自定义主页 XAML 被视为不受信任内容;禁止引用 WPF Behaviors 中可调用方法/改写属性的通用动作。 | ||
| foreach (var blockedXamlNamespace in new[] |
Member
There was a problem hiding this comment.
i think u can make a static readonly array
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
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.
Motivation
Microsoft.Xaml.Behaviors.Wpfand aTriggerAction-derived animation action (RunAnimationAction), which expands the XAML surface and enables generic behavior actions that can invoke methods from untrusted homepage XAML.CallMethodAction/ChangePropertyAction) that could enable arbitrary method/property invocation.Description
ModBase.GetObjectFromXML(string)that scans the XAML string and throwsUnauthorizedAccessExceptionwhen it contains any ofMicrosoft.Xaml.Behaviors,Microsoft.Xaml.Behaviors.Core,Microsoft.Xaml.Interactions.Core, orhttp://schemas.microsoft.com/xaml/behaviors.XamlXmlReader-based type/member blacklist and continue to load viaXamlReader.Loadwhen checks pass.Testing
UnauthorizedAccessExceptionpath.git diff --checkto ensure no style/whitespace issues were introduced.dotnet build 'Plain Craft Launcher 2/Plain Craft Launcher 2.csproj' -c Debug -v:minimalin this environment becausedotnetis not installed, so compilation was not validated here.Codex Task
Summary by Sourcery
错误修复:
Microsoft.Xaml.Behaviors或相关行为命名空间时,在解析之前抛出UnauthorizedAccessException,以防止加载这些 XAML。Original summary in English
Summary by Sourcery
Bug Fixes: