From 0e96dd08a2eb3fb23bc68699f4f846858fbd0064 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 10 Nov 2026 07:48:39 -0500 Subject: [PATCH] fix(scripting): deny System.AppContext and other process-wide mutators in the script sandbox (lmxopcua#792) 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 --- docs/VirtualTags.md | 2 +- ...-09-out-of-process-script-runner-design.md | 11 ++- .../ForbiddenTypeAnalyzer.cs | 91 +++++++++++++++---- .../ScriptSandboxHardeningTests.cs | 64 +++++++++++++ .../ShadowModeTests.cs | 20 +++- 5 files changed, 161 insertions(+), 27 deletions(-) diff --git a/docs/VirtualTags.md b/docs/VirtualTags.md index 40ae5fd9..be47d254 100644 --- a/docs/VirtualTags.md +++ b/docs/VirtualTags.md @@ -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) diff --git a/docs/plans/2026-11-09-out-of-process-script-runner-design.md b/docs/plans/2026-11-09-out-of-process-script-runner-design.md index d1938e85..571a75c3 100644 --- a/docs/plans/2026-11-09-out-of-process-script-runner-design.md +++ b/docs/plans/2026-11-09-out-of-process-script-runner-design.md @@ -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 diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ForbiddenTypeAnalyzer.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ForbiddenTypeAnalyzer.cs index 5f7d8140..43358030 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ForbiddenTypeAnalyzer.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ForbiddenTypeAnalyzer.cs @@ -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"), ]; /// @@ -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", ]; /// @@ -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 } } + /// + /// Member deny-list check on its own (no type / namespace rule): rejects a method or property whose declaring + /// type and name are listed in . Exact declaring-type match, so an inherited + /// harmless member is never flagged. + /// + private static bool TryRejectMember(ISymbol? symbol, TextSpan span, string ns, List 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 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). diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptSandboxHardeningTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptSandboxHardeningTests.cs index 5c932832..05da1e52 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptSandboxHardeningTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptSandboxHardeningTests.cs @@ -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.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(); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ScriptWorker.Tests/ShadowModeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ScriptWorker.Tests/ShadowModeTests.cs index 3655fa57..e9a51a88 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ScriptWorker.Tests/ShadowModeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ScriptWorker.Tests/ShadowModeTests.cs @@ -24,8 +24,13 @@ namespace ZB.MOM.WW.OtOpcUa.ScriptWorker.Tests; [Trait("Category", "ScriptWorker")] public sealed class ShadowModeTests { - /// True only inside the ScriptWorker (its base directory is script-worker/): a test-only divergence. - private const string InWorker = "System.AppContext.BaseDirectory.Contains(\"script-worker\")"; + /// + /// True only inside the ScriptWorker: a test-only divergence that needs no sandbox-forbidden API. The worker runs + /// with InvariantGlobalization (ordinal string comparison) while the test host does linguistic (ICU/NLS) + /// comparison, so a culture-sensitive string.Compare("a", "B") is negative in the host and positive in the + /// worker. pins the premise. + /// + 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() {