fix(scripting): deny System.AppContext and other process-wide mutators in the script sandbox (lmxopcua#792)
v2-ci / unit-tests (push) Waiting to run
v2-ci / unit-tests (push) Waiting to run
ForbiddenTypeAnalyzer now denies System.AppContext, System.Console, GCSettings, ProfileOptimization,
EncodingProvider/CodePagesEncodingProvider, TypeDescriptor and CryptoConfig as types, and the ambient
CultureInfo.{DefaultThreadCurrentCulture,DefaultThreadCurrentUICulture,CurrentCulture,CurrentUICulture}
and TimeZoneInfo.ClearCachedData members (member deny-list now covers properties and member-access reads).
Shadow-mode tests diverge via culture-sensitive string.Compare (worker is InvariantGlobalization) instead of
AppContext.BaseDirectory, with a premise test. Mutation-proved: 21 new cases fail with the analyzer reverted.
Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
+1
-1
@@ -33,7 +33,7 @@ User scripts are compiled via `Microsoft.CodeAnalysis.CSharp` (regular compiler,
|
||||
|
||||
`ForbiddenTypeAnalyzer.ForbiddenNamespacePrefixes` denies `System.IO`, `System.Net`, `System.Diagnostics`, `System.Reflection`, `System.Threading` (the whole namespace — `ForbiddenTypeAnalyzer.AllowedTypesInForbiddenNamespaces` lets only `CancellationToken`, `Interlocked` and `Volatile` back in), `System.Threading.Tasks`, `System.Runtime.InteropServices`, `System.Runtime.Loader`, and `Microsoft.Win32`. Matching is by prefix against the resolved symbol's containing namespace, so `System.Net` catches `System.Net.Http.HttpClient` and every subnamespace. `System.Threading.Tasks` is denied because scripts are synchronous predicates with no legitimate need to start background tasks — a `Task.Run` fan-out would outlive the per-evaluation timeout entirely (Core.Scripting-003). `System.Runtime.Loader` is denied to block `AssemblyLoadContext` / `AssemblyDependencyResolver` — arbitrary DLL loads into the host process (Core.Scripting-012).
|
||||
|
||||
`ForbiddenTypeAnalyzer.ForbiddenFullTypeNames` denies type-granularly: `System.Environment`, `System.AppDomain`, `System.GC`, `System.Activator`, `System.Runtime.CompilerServices.Unsafe`, and `System.Runtime.CompilerServices.RuntimeHelpers`. These live in namespaces that are otherwise allowed — `System` (primitives, `Math`) and `System.Runtime.CompilerServices` (compile-time attributes such as `CallerMemberNameAttribute`) — so a namespace-prefix rule cannot reach them. `Environment.Exit` / `FailFast` terminate the host process outright (Core.Scripting-001); `Unsafe.As` bypasses the type system; `RuntimeHelpers.GetUninitializedObject` builds an instance of any type without a constructor. `Thread` / `ThreadPool` / `Timer` / `SemaphoreSlim` and the rest of `System.Threading` are now caught by the namespace prefix above instead of being listed one by one — a script is a synchronous expression with no use for background work or blocking primitives (Core.Scripting-010 / -012, review R05:10). `ForbiddenTypeAnalyzer.ForbiddenMembers` additionally denies `System.Type`'s by-name reflection entry points (`GetType(string)`, `GetMethod(s)`, `InvokeMember`, and the `GetProperty`/`GetField`/`GetMember`/`GetConstructor`/`GetEvent` families), which would otherwise reach a forbidden type without naming it syntactically.
|
||||
`ForbiddenTypeAnalyzer.ForbiddenFullTypeNames` denies type-granularly: `System.Environment`, `System.AppDomain`, `System.GC`, `System.Activator`, `System.Runtime.CompilerServices.Unsafe`, `System.Runtime.CompilerServices.RuntimeHelpers`, and (lmxopcua#792, process-wide runtime configuration) `System.AppContext` (`SetSwitch`/`SetData` flip BCL switches for the whole process; `GetData`/`BaseDirectory` leak host paths), `System.Console` (redirects/writes the process streams — log through `ctx.Logger`), `System.Runtime.GCSettings`, `System.Runtime.ProfileOptimization`, `System.Text.EncodingProvider`/`CodePagesEncodingProvider`, `System.ComponentModel.TypeDescriptor` and `System.Security.Cryptography.CryptoConfig`. These live in namespaces that are otherwise allowed — `System` (primitives, `Math`) and `System.Runtime.CompilerServices` (compile-time attributes such as `CallerMemberNameAttribute`) — so a namespace-prefix rule cannot reach them. `Environment.Exit` / `FailFast` terminate the host process outright (Core.Scripting-001); `Unsafe.As` bypasses the type system; `RuntimeHelpers.GetUninitializedObject` builds an instance of any type without a constructor. `Thread` / `ThreadPool` / `Timer` / `SemaphoreSlim` and the rest of `System.Threading` are now caught by the namespace prefix above instead of being listed one by one — a script is a synchronous expression with no use for background work or blocking primitives (Core.Scripting-010 / -012, review R05:10). `ForbiddenTypeAnalyzer.ForbiddenMembers` additionally denies `System.Type`'s by-name reflection entry points (`GetType(string)`, `GetMethod(s)`, `InvokeMember`, and the `GetProperty`/`GetField`/`GetMember`/`GetConstructor`/`GetEvent` families), which would otherwise reach a forbidden type without naming it syntactically, and the ambient-state members `CultureInfo.DefaultThreadCurrentCulture` / `DefaultThreadCurrentUICulture` / `CurrentCulture` / `CurrentUICulture` and `TimeZoneInfo.ClearCachedData` (use `CultureInfo.InvariantCulture` / `GetCultureInfo(...)`). Scripts running in the out-of-process ScriptWorker are rejected by the same analyzer at compile.
|
||||
|
||||
#### Known resource limits (accepted trade-offs)
|
||||
|
||||
|
||||
@@ -1133,8 +1133,9 @@ other work on the box, 10 s phases):**
|
||||
hangs, blocks its caller or throws never reaches the caller and is never a mismatch; MaxInFlight drop accounting;
|
||||
inputs copied before `Mirror` returns.
|
||||
- `ShadowModeTests` (real worker): eleven scripts over short / double / string / arrays / DateTime / bool / status /
|
||||
a throw publish exactly what InProcess publishes and match; a deliberately divergent script (it reads
|
||||
`AppContext.BaseDirectory`, so it answers differently in the worker) and a `decimal` result are counted against
|
||||
a throw publish exactly what InProcess publishes and match; a deliberately divergent script (it does a culture-sensitive
|
||||
`string.Compare("a", "B")`: the worker runs `InvariantGlobalization` so it compares ordinally and answers differently;
|
||||
originally it read `AppContext.BaseDirectory`, which lmxopcua#792 then denied) and a `decimal` result are counted against
|
||||
exactly those two scripts (`value`, `unsupported_type`). A runaway that only runs away in the worker is killed twice
|
||||
and quarantined **there** while every published value stays the in-process one, no published evaluation waits more
|
||||
than 250 ms, and its sibling on the killed shard (a victim, not compared) has no mismatch. Alarms: the emission
|
||||
@@ -1146,8 +1147,10 @@ other work on the box, 10 s phases):**
|
||||
VirtualTags 16, Calculation 84, ScriptWorker 160/4 skip, Host 275, Runtime 1002/3 skip, OpcUaServer.IntegrationTests
|
||||
133. windev (Windows VM, scratch dir under `C:\build`): ScriptWorker suite 160 passed / 4 skipped, 0 warnings. The first
|
||||
windev run caught a test bug (a `DateTime` seconds argument past 59 once the slower VM looped longer), fixed before the commit.
|
||||
- Found while writing them: scripts can reach `System.AppContext` (`SetSwitch` / `SetData` / `GetData`) →
|
||||
lmxopcua#792.
|
||||
- Found while writing them: scripts could reach `System.AppContext` (`SetSwitch` / `SetData` / `GetData`) →
|
||||
lmxopcua#792, fixed: `System.AppContext`, `System.Console`, `GCSettings`, `ProfileOptimization`, `EncodingProvider`,
|
||||
`TypeDescriptor`, `CryptoConfig` and the ambient `CultureInfo` / `TimeZoneInfo.ClearCachedData` members are now denied
|
||||
by `ForbiddenTypeAnalyzer` (in-process and in the worker, one analyzer).
|
||||
|
||||
**docker-dev drill** (image `otopcua-host:w34-shadow` built from this branch; `central-1`/`central-2` only; drill rows
|
||||
`w34-*` inserted by SQL: VTs `w34mul` (HR5×3), `w34raw` (`var v = …HR5.Value; return v;` — compiled, returns the boxed
|
||||
|
||||
@@ -129,6 +129,18 @@ public static class ForbiddenTypeAnalyzer
|
||||
("System.Type", "GetEvent"),
|
||||
("System.Type", "GetEvents"),
|
||||
("System.Type", "GetDefaultMembers"),
|
||||
|
||||
// Process-wide mutators on otherwise-useful types (lmxopcua#792). CultureInfo itself must stay
|
||||
// reachable (CultureInfo.InvariantCulture is the right way to format), but its ambient-culture
|
||||
// properties are process / pool-thread state: Default* apply to every thread created afterwards, and
|
||||
// CurrentCulture / CurrentUICulture stick to the pooled thread the script ran on. The whole property is
|
||||
// denied (read and write) — a script that wants a culture names InvariantCulture or CultureInfo.GetCultureInfo.
|
||||
("System.Globalization.CultureInfo", "DefaultThreadCurrentCulture"),
|
||||
("System.Globalization.CultureInfo", "DefaultThreadCurrentUICulture"),
|
||||
("System.Globalization.CultureInfo", "CurrentCulture"),
|
||||
("System.Globalization.CultureInfo", "CurrentUICulture"),
|
||||
// Drops the process-wide time-zone cache.
|
||||
("System.TimeZoneInfo", "ClearCachedData"),
|
||||
];
|
||||
|
||||
/// <summary>
|
||||
@@ -188,6 +200,30 @@ public static class ForbiddenTypeAnalyzer
|
||||
// included — without a constructor or a syntactic type name. Same namespace caveat
|
||||
// as Unsafe above, so type-granular.
|
||||
"System.Runtime.CompilerServices.RuntimeHelpers",
|
||||
// lmxopcua#792 — process-wide runtime configuration. System.AppContext.SetSwitch / SetData flip
|
||||
// BCL compatibility switches (serialization, networking, globalization) for the whole process and
|
||||
// GetData / BaseDirectory leak the host's install path and TRUSTED_PLATFORM_ASSEMBLIES. The whole
|
||||
// type is denied (same class as Environment / AppDomain, which are already denied); TryGetSwitch is
|
||||
// read-only but has no script use either.
|
||||
"System.AppContext",
|
||||
// Console.SetOut / SetError / SetIn / Out / Write* replace or write the process's standard streams —
|
||||
// in the ScriptWorker stdout/stderr belong to the launcher, in the host they belong to the service
|
||||
// log capture. Scripts log through ctx.Logger.
|
||||
"System.Console",
|
||||
// GCSettings.LatencyMode / LargeObjectHeapCompactionMode are process-wide GC tuning knobs.
|
||||
"System.Runtime.GCSettings",
|
||||
// ProfileOptimization.SetProfileRoot / StartProfile redirect the JIT profile root (a file-writing,
|
||||
// process-wide setting).
|
||||
"System.Runtime.ProfileOptimization",
|
||||
// EncodingProvider registration is process-wide (Encoding.RegisterProvider takes an EncodingProvider).
|
||||
"System.Text.EncodingProvider",
|
||||
"System.Text.CodePagesEncodingProvider",
|
||||
// TypeDescriptor.AddProvider / AddAttributes change type metadata for the whole process, and
|
||||
// CreateInstance builds a type by name.
|
||||
"System.ComponentModel.TypeDescriptor",
|
||||
// CryptoConfig.AddAlgorithm / AddOID mutate the process-wide algorithm map, and CreateFromName builds
|
||||
// a type by name (an Activator-equivalent).
|
||||
"System.Security.Cryptography.CryptoConfig",
|
||||
];
|
||||
|
||||
/// <summary>
|
||||
@@ -252,6 +288,11 @@ public static class ForbiddenTypeAnalyzer
|
||||
// MemberAccess "System.IO.File" resolves to the File type symbol
|
||||
// whose containing namespace is System.IO, triggering a rejection.
|
||||
CheckSymbol(semantic.GetSymbolInfo(mem.Expression).Symbol, mem.Expression.Span, rejections);
|
||||
// The accessed member itself: a property such as CultureInfo.DefaultThreadCurrentCulture is
|
||||
// never an invocation, so the member deny-list must run here (member-only — resolving the
|
||||
// type of an inherited member would flag harmless reads, see the class remarks).
|
||||
var accessed = semantic.GetSymbolInfo(mem).Symbol;
|
||||
TryRejectMember(accessed, mem.Span, accessed?.ContainingNamespace?.ToDisplayString() ?? string.Empty, rejections);
|
||||
break;
|
||||
case IdentifierNameSyntax id when node.Parent is not MemberAccessExpressionSyntax:
|
||||
CheckSymbol(semantic.GetSymbolInfo(id).Symbol, id.Span, rejections);
|
||||
@@ -294,6 +335,34 @@ public static class ForbiddenTypeAnalyzer
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Member deny-list check on its own (no type / namespace rule): rejects a method or property whose declaring
|
||||
/// type and name are listed in <see cref="ForbiddenMembers"/>. Exact declaring-type match, so an inherited
|
||||
/// harmless member is never flagged.
|
||||
/// </summary>
|
||||
private static bool TryRejectMember(ISymbol? symbol, TextSpan span, string ns, List<ForbiddenTypeRejection> rejections)
|
||||
{
|
||||
if (symbol is not (IMethodSymbol or IPropertySymbol)) return false;
|
||||
|
||||
var declaring = symbol.ContainingType.OriginalDefinition.ToDisplayString(
|
||||
SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(
|
||||
SymbolDisplayGlobalNamespaceStyle.Omitted));
|
||||
foreach (var (type, member) in ForbiddenMembers)
|
||||
{
|
||||
if (declaring != type || symbol.Name != member) continue;
|
||||
if (rejections.Any(r => r.Span == span && r.TypeName == $"{type}.{member}")) return true;
|
||||
rejections.Add(new ForbiddenTypeRejection(
|
||||
Span: span,
|
||||
TypeName: $"{type}.{member}",
|
||||
Namespace: ns,
|
||||
Message: $"Member '{type}.{member}' is on the sandbox forbidden-member deny-list: " +
|
||||
"it obtains a type or member by name or changes process-wide state, which bypasses the sandbox " +
|
||||
"(see ForbiddenTypeAnalyzer.ForbiddenMembers for the authoritative list)."));
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private static void CheckSymbol(ISymbol? symbol, TextSpan span, List<ForbiddenTypeRejection> rejections)
|
||||
{
|
||||
if (symbol is null) return;
|
||||
@@ -327,26 +396,8 @@ public static class ForbiddenTypeAnalyzer
|
||||
|
||||
// Member deny-list first: the containing type (System.Type) is allowed, so neither list
|
||||
// below would ever look at the member name.
|
||||
if (symbol is IMethodSymbol method)
|
||||
{
|
||||
var declaring = method.ContainingType.OriginalDefinition.ToDisplayString(
|
||||
SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(
|
||||
SymbolDisplayGlobalNamespaceStyle.Omitted));
|
||||
foreach (var (type, member) in ForbiddenMembers)
|
||||
{
|
||||
if (declaring == type && method.Name == member)
|
||||
{
|
||||
rejections.Add(new ForbiddenTypeRejection(
|
||||
Span: span,
|
||||
TypeName: $"{type}.{member}",
|
||||
Namespace: ns,
|
||||
Message: $"Member '{type}.{member}' is on the sandbox forbidden-member deny-list: " +
|
||||
"it obtains a type or member by name, which bypasses the type allow-list " +
|
||||
"(see ForbiddenTypeAnalyzer.ForbiddenMembers for the authoritative list)."));
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (TryRejectMember(symbol, span, ns, rejections))
|
||||
return;
|
||||
|
||||
// A type on the allow-list stays reachable even though its namespace is denied; it is still
|
||||
// subject to the type-granular deny-list below (nothing is on both today).
|
||||
|
||||
@@ -145,4 +145,68 @@ public sealed class ScriptSandboxHardeningTests
|
||||
"var o = System.Runtime.CompilerServices.MethodImplOptions.NoInlining; return (int)o;");
|
||||
(await ev.RunAsync(new FakeScriptContext(), TestContext.Current.CancellationToken)).ShouldBe(8);
|
||||
}
|
||||
|
||||
// ── Process-wide mutators (lmxopcua#792) ─────────────────────────────────────────────────
|
||||
|
||||
[Theory]
|
||||
[InlineData("System.AppContext.SetSwitch(\"System.Globalization.Invariant\", true);")]
|
||||
[InlineData("System.AppContext.SetData(\"k\", \"v\");")]
|
||||
[InlineData("var d = System.AppContext.GetData(\"TRUSTED_PLATFORM_ASSEMBLIES\");")]
|
||||
[InlineData("var b = System.AppContext.BaseDirectory;")]
|
||||
[InlineData("System.AppContext.TryGetSwitch(\"x\", out var v);")]
|
||||
[InlineData("var t = typeof(System.AppContext);")]
|
||||
[InlineData("System.Console.SetOut(System.IO.TextWriter.Null);")]
|
||||
[InlineData("System.Console.WriteLine(\"x\");")]
|
||||
[InlineData("System.Runtime.GCSettings.LatencyMode = System.Runtime.GCLatencyMode.LowLatency;")]
|
||||
[InlineData("System.Runtime.ProfileOptimization.SetProfileRoot(\"/tmp\");")]
|
||||
[InlineData("System.Text.Encoding.RegisterProvider(System.Text.CodePagesEncodingProvider.Instance);")]
|
||||
[InlineData("System.ComponentModel.TypeDescriptor.AddAttributes(typeof(string));")]
|
||||
[InlineData("System.Security.Cryptography.CryptoConfig.AddAlgorithm(typeof(string), \"x\");")]
|
||||
[InlineData("System.Security.Cryptography.CryptoConfig.CreateFromName(\"x\");")]
|
||||
[InlineData("System.Globalization.CultureInfo.DefaultThreadCurrentCulture = System.Globalization.CultureInfo.InvariantCulture;")]
|
||||
[InlineData("System.Globalization.CultureInfo.DefaultThreadCurrentUICulture = System.Globalization.CultureInfo.InvariantCulture;")]
|
||||
[InlineData("System.Globalization.CultureInfo.CurrentCulture = System.Globalization.CultureInfo.InvariantCulture;")]
|
||||
[InlineData("System.Globalization.CultureInfo.CurrentUICulture = System.Globalization.CultureInfo.InvariantCulture;")]
|
||||
[InlineData("var c = System.Globalization.CultureInfo.CurrentCulture;")]
|
||||
[InlineData("System.TimeZoneInfo.ClearCachedData();")]
|
||||
[InlineData("System.AppDomain.CurrentDomain.SetData(\"k\", \"v\");")]
|
||||
[InlineData("System.Environment.SetEnvironmentVariable(\"k\", \"v\");")]
|
||||
[InlineData("System.GC.Collect();")]
|
||||
[InlineData("System.Threading.ThreadPool.SetMinThreads(1, 1);")]
|
||||
[InlineData("System.Threading.Thread.CurrentThread.CurrentCulture = System.Globalization.CultureInfo.InvariantCulture;")]
|
||||
public void Rejects_process_wide_mutators(string statement)
|
||||
{
|
||||
var ex = Rejects(statement + " return 0;");
|
||||
ex.Rejections.ShouldNotBeEmpty();
|
||||
ex.Message.ShouldContain("sandbox");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AppContext_rejection_names_the_type()
|
||||
{
|
||||
var ex = Rejects("System.AppContext.SetSwitch(\"a\", true); return 0;");
|
||||
ex.Rejections.ShouldContain(r => r.Message.Contains("System.AppContext") && r.Message.Contains("forbidden-type"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CultureInfo_member_rejection_names_the_member()
|
||||
{
|
||||
var ex = Rejects("System.Globalization.CultureInfo.DefaultThreadCurrentCulture = null; return 0;");
|
||||
ex.Rejections.ShouldContain(r => r.TypeName == "System.Globalization.CultureInfo.DefaultThreadCurrentCulture");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Legitimate_culture_and_time_usage_still_compiles()
|
||||
{
|
||||
// CultureInfo itself, its Invariant / named-lookup entry points and ordinary formatting stay usable.
|
||||
ScriptEvaluator<FakeScriptContext, int>.Compile(
|
||||
"""
|
||||
var inv = System.Globalization.CultureInfo.InvariantCulture;
|
||||
var de = System.Globalization.CultureInfo.GetCultureInfo("de-DE");
|
||||
var s = (1.5).ToString("F2", inv) + 2.5.ToString(de) + string.Format(inv, "{0}", 3);
|
||||
var tz = System.TimeZoneInfo.Utc;
|
||||
var n = System.TimeZoneInfo.ConvertTimeFromUtc(System.DateTime.UtcNow, tz);
|
||||
return s.Length + 1 + System.Math.Abs(-1);
|
||||
""").ShouldNotBeNull();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,8 +24,13 @@ namespace ZB.MOM.WW.OtOpcUa.ScriptWorker.Tests;
|
||||
[Trait("Category", "ScriptWorker")]
|
||||
public sealed class ShadowModeTests
|
||||
{
|
||||
/// <summary>True only inside the ScriptWorker (its base directory is <c>script-worker/</c>): a test-only divergence.</summary>
|
||||
private const string InWorker = "System.AppContext.BaseDirectory.Contains(\"script-worker\")";
|
||||
/// <summary>
|
||||
/// True only inside the ScriptWorker: a test-only divergence that needs no sandbox-forbidden API. The worker runs
|
||||
/// with <c>InvariantGlobalization</c> (ordinal string comparison) while the test host does linguistic (ICU/NLS)
|
||||
/// comparison, so a culture-sensitive <c>string.Compare("a", "B")</c> is negative in the host and positive in the
|
||||
/// worker. <see cref="Culture_sensitive_comparison_really_diverges_between_host_and_worker"/> pins the premise.
|
||||
/// </summary>
|
||||
private const string InWorker = "(string.Compare(\"a\", \"B\") > 0)";
|
||||
|
||||
private static CancellationToken Ct => TestContext.Current.CancellationToken;
|
||||
|
||||
@@ -60,6 +65,17 @@ public sealed class ShadowModeTests
|
||||
&& System.Collections.StructuralComparisons.StructuralEqualityComparer.Equals(a.Value, b.Value)
|
||||
&& a.Value?.GetType() == b.Value?.GetType();
|
||||
|
||||
[Fact]
|
||||
public async Task Culture_sensitive_comparison_really_diverges_between_host_and_worker()
|
||||
{
|
||||
// Premise of InWorker: the worker is a different process whose globalization mode differs from the host's.
|
||||
string.Compare("a", "B").ShouldBeLessThan(0, "the test host compares linguistically");
|
||||
await using var pool = Scripts.Runner(Scripts.Options());
|
||||
var r = await pool.RunAsync(Scripts.Vt("probe", $"return {InWorker} ? 2 : 1;"), Ct);
|
||||
r.Status.ShouldBe(ScriptRunStatus.Ok, r.Reason);
|
||||
r.Value.ShouldBe(2, "the worker (InvariantGlobalization) compares ordinally");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Ordinary_scripts_match_and_a_divergent_and_an_unsupported_one_are_counted_against_exactly_those_scripts()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user