Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 83 additions & 4 deletions PCL.Core/UI/IconManager.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
using System;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Xaml;
using System.Windows;
using System.Windows.Markup;
using System.Windows.Controls;
using System.Windows.Data;
using WpfXamlReader = System.Windows.Markup.XamlReader;

namespace PCL.Core.UI;

Expand Down Expand Up @@ -55,7 +61,8 @@ public static bool TryLoadIconFromXaml(string xamlString, out UIElement? icon) {
}

try {
icon = (UIElement)XamlReader.Parse(xamlString);
ValidateSafeLooseXaml(xamlString);
icon = (UIElement)WpfXamlReader.Parse(xamlString);
return true;
}
catch (Exception) {
Expand All @@ -75,10 +82,82 @@ public static bool LoadIconFromXaml(string xamlString, out UIElement? icon) {
throw new InvalidOperationException("XAML 解析需要在 UI 线程执行。");
}

icon = (UIElement)XamlReader.Parse(xamlString);
ValidateSafeLooseXaml(xamlString);
icon = (UIElement)WpfXamlReader.Parse(xamlString);
return true;
}

private static readonly Type[] DisallowedLooseXamlTypes =
[
typeof(WebBrowser),
typeof(Frame),
typeof(MediaElement),
typeof(ObjectDataProvider),
typeof(XmlDataProvider),
typeof(WpfXamlReader),
typeof(Window),
typeof(Process),
typeof(ProcessStartInfo)
];

private static readonly string[] DisallowedLooseXamlMembers =
[
"Code",
"FactoryMethod",
"Static"
];

private static readonly string[] DisallowedLooseXamlAssemblies =
[
"Microsoft.Xaml.Behaviors"
];

private static void ValidateSafeLooseXaml(string xamlString) {
using var stream = new MemoryStream(System.Text.Encoding.UTF8.GetBytes(xamlString));
using var reader = new XamlXmlReader(stream);

while (reader.Read()) {
if (reader.Type?.UnderlyingType is { } nodeType) {
ValidateLooseXamlType(nodeType);
}

if (reader.Member is { } member) {
if (DisallowedLooseXamlMembers.Contains(member.Name)) {
throw new UnauthorizedAccessException($"不允许使用 {member.Name} 成员。");
}

if (member.DeclaringType?.UnderlyingType is { } declaringType) {
ValidateLooseXamlType(declaringType);
}
}

if (reader.Value is string value) {
ValidateLooseXamlValue(value);
}
}
}

private static void ValidateLooseXamlType(Type type) {
if (DisallowedLooseXamlTypes.Any(disallowedType => disallowedType.IsAssignableFrom(type))) {
throw new UnauthorizedAccessException($"不允许使用 {type.Name} 类型。");
}

var assemblyName = type.Assembly.GetName().Name;
if (assemblyName is not null && DisallowedLooseXamlAssemblies.Any(disallowedAssembly => assemblyName.StartsWith(disallowedAssembly, StringComparison.Ordinal))) {
throw new UnauthorizedAccessException($"不允许使用 {assemblyName} 程序集中的类型。");
}
}

private static void ValidateLooseXamlValue(string value) {
if (DisallowedLooseXamlTypes.Any(disallowedType => string.Equals(value, disallowedType.Name, StringComparison.Ordinal))) {
throw new UnauthorizedAccessException($"不允许使用 {value} 值。");
}

if (DisallowedLooseXamlAssemblies.Any(disallowedAssembly => value.Contains(disallowedAssembly, StringComparison.Ordinal))) {
throw new UnauthorizedAccessException($"不允许引用 {value}。");
Comment on lines +152 to +157

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): 基于值的类型检查可能会漏掉通过全限定名或标记扩展语法引用的危险类型。

ValidateLooseXamlValue 只把 valuedisallowedType.Name 做比较,因此像 System.Diagnostics.Process{x:Type Process} 或其他带命名空间/标记形式的输入不会被拦截。如果这个检查是作为字符串值的安全兜底机制,建议至少使用 EndsWith(disallowedType.Name, StringComparison.Ordinal),或者在比较前对常见模式做归一化处理,避免危险类型通过限定名或标记语法绕过过滤。

Original comment in English

🚨 issue (security): The value-based type check may miss dangerous types referenced via fully qualified or markup-extension syntax.

ValidateLooseXamlValue compares value only to disallowedType.Name, so inputs like System.Diagnostics.Process, {x:Type Process}, or other namespace/markup forms will not be caught. If this check is intended as a safety net for string-only values, consider at least using EndsWith(disallowedType.Name, StringComparison.Ordinal) or normalizing common patterns before comparison so dangerous types cannot bypass the filter via qualified or markup syntax.

}
}

public event PropertyChangedEventHandler? PropertyChanged;
protected void OnPropertyChanged(string propertyName) =>
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
Expand Down
13 changes: 12 additions & 1 deletion Plain Craft Launcher 2/Modules/Base/ModBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3223,7 +3223,8 @@ public static object GetObjectFromXML(string str)
foreach (var blackListType in new[]
{
typeof(WebBrowser), typeof(Frame), typeof(MediaElement), typeof(ObjectDataProvider),
typeof(XamlReader), typeof(Window), typeof(XmlDataProvider)
typeof(XamlReader), typeof(Window), typeof(XmlDataProvider),
typeof(Process), typeof(ProcessStartInfo)
})
{
if (reader.Type is not null && blackListType.IsAssignableFrom(reader.Type.UnderlyingType))
Expand All @@ -3232,6 +3233,16 @@ public static object GetObjectFromXML(string str)
throw new UnauthorizedAccessException($"不允许使用 {blackListType.Name} 值。");
}

foreach (var blackListAssembly in new[] { "Microsoft.Xaml.Behaviors" })
{
if (reader.Type?.UnderlyingType?.Assembly.GetName().Name?.StartsWith(blackListAssembly, StringComparison.Ordinal) == true)
throw new UnauthorizedAccessException($"不允许使用 {blackListAssembly} 程序集中的类型。");
if (reader.Member?.DeclaringType?.UnderlyingType?.Assembly.GetName().Name?.StartsWith(blackListAssembly, StringComparison.Ordinal) == true)
throw new UnauthorizedAccessException($"不允许使用 {blackListAssembly} 程序集中的成员。");
if (reader.Value is string value && value.Contains(blackListAssembly, StringComparison.Ordinal))
throw new UnauthorizedAccessException($"不允许引用 {blackListAssembly} 程序集。");
}

foreach (var blackListMember in new[] { "Code", "FactoryMethod", "Static" })
if (reader.Member is not null && (reader.Member.Name ?? "") == (blackListMember ?? ""))
throw new UnauthorizedAccessException($"不允许使用 {blackListMember} 成员。");
Expand Down
Loading