diff --git a/src/testengine.provider.canvas.tests/CanvasAppSdkInjectionTests.cs b/src/testengine.provider.canvas.tests/CanvasAppSdkInjectionTests.cs new file mode 100644 index 000000000..8d61b077a --- /dev/null +++ b/src/testengine.provider.canvas.tests/CanvasAppSdkInjectionTests.cs @@ -0,0 +1,178 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Reflection; +using Jint; +using Newtonsoft.Json; + +namespace Microsoft.PowerApps.TestEngine.Tests.PowerApps +{ + /// + /// Regression tests for the CWE-94 code injection fix in CanvasAppSdk.js. + /// + /// interactWithControl() builds a script string that executePublishedAppScript() marshals into + /// the published app and evaluates there. The property name comes from the test plan + /// (.fx.yaml), so it has to be escaped with JSON.stringify() instead of being wrapped in hand + /// written quotes, otherwise a crafted name can close the object literal and the argument list + /// and append arbitrary statements to the script that crosses into the app. + /// + public class CanvasAppSdkInjectionTests + { + private const string SdkResourceName = "testengine.provider.canvas.tests.CanvasAppSdk.js"; + + /// + /// Property name that closes the generated object literal and the enclosing argument list, + /// appends a statement, then reopens both so the injected script still parses. + /// + private const string InjectionPropertyName = "a\":0});injected = true;({\"b"; + + private static string GetCanvasSdkSource() + { + var assembly = Assembly.GetExecutingAssembly(); + + using (var stream = assembly.GetManifestResourceStream(SdkResourceName)) + { + Assert.True(stream != null, $"Embedded resource {SdkResourceName} was not found"); + + using (var reader = new StreamReader(stream)) + { + return reader.ReadToEnd(); + } + } + } + + /// + /// Loads the shipped SDK and replaces executePublishedAppScript so the script that would be + /// sent to the published app can be inspected instead of dispatched. + /// + /// + /// Object.values() always returns an array, so the scalar branch of interactWithControl is + /// only reachable when isArray is stubbed out. + /// + private static Engine CreateSdkEngine(bool forceScalarBranch) + { + var engine = new Engine(); + + // debugInfo is evaluated while the SDK loads and reads the published app telemetry + engine.Execute("var Core = { Telemetry: {} };"); + engine.Execute("var capturedScript = null; var injected = false; var probedKeys = null;"); + + engine.Execute(GetCanvasSdkSource()); + + engine.Execute("executePublishedAppScript = function (scriptToExecute) { capturedScript = scriptToExecute; return scriptToExecute; };"); + + if (forceScalarBranch) + { + engine.Execute("isArray = function () { return false; };"); + } + + return engine; + } + + private static string ItemPathJson(string propertyName) + { + return JsonConvert.SerializeObject(new { controlName = "Label1", propertyName }); + } + + private static string BuildScript(Engine engine, string propertyName, string valueLiteral) + { + engine.Execute($"interactWithControl({ItemPathJson(propertyName)}, {valueLiteral});"); + + return engine.Evaluate("capturedScript").AsString(); + } + + /// + /// Stands in for the published app side implementation so the captured script can be run. + /// + private static void InstallProbe(Engine engine) + { + engine.Execute("interactWithControl = function (itemPath, value) { probedKeys = Object.keys(value); return true; };"); + } + + [Theory] + [InlineData(true, "{ Value: 1 }")] + [InlineData(false, "1")] + public void InteractWithControlEscapesPropertyNameInGeneratedScript(bool useArrayBranch, string valueLiteral) + { + // Arrange + var engine = CreateSdkEngine(forceScalarBranch: !useArrayBranch); + + // Act + var script = BuildScript(engine, InjectionPropertyName, valueLiteral); + + // Assert - the quote that would terminate the key is escaped + Assert.Contains("a\\\":0});injected", script); + + // Assert - the payload stays inert when the script reaches the published app + InstallProbe(engine); + engine.Execute($"eval({JsonConvert.SerializeObject(script)});"); + Assert.False(engine.Evaluate("injected").AsBoolean()); + } + + [Theory] + [InlineData(true, "{ Value: 1 }")] + [InlineData(false, "1")] + public void InteractWithControlDeliversPropertyNameAsSingleKey(bool useArrayBranch, string valueLiteral) + { + // Arrange + var engine = CreateSdkEngine(forceScalarBranch: !useArrayBranch); + var script = BuildScript(engine, InjectionPropertyName, valueLiteral); + + InstallProbe(engine); + + // Act + engine.Execute($"eval({JsonConvert.SerializeObject(script)});"); + + // Assert - the whole payload arrives as one inert key rather than as extra statements + Assert.False(engine.Evaluate("injected").AsBoolean()); + Assert.Equal(1, (int)engine.Evaluate("probedKeys.length").AsNumber()); + Assert.Equal(InjectionPropertyName, engine.Evaluate("probedKeys[0]").AsString()); + } + + [Theory] + [InlineData(true, "{ Value: 1 }")] + [InlineData(false, "1")] + public void InteractWithControlIsUnchangedForOrdinaryPropertyNames(bool useArrayBranch, string valueLiteral) + { + // Arrange + var engine = CreateSdkEngine(forceScalarBranch: !useArrayBranch); + + // Act + var script = BuildScript(engine, "Text", valueLiteral); + + // Assert - escaping the key must not alter the script produced for a normal plan + var expected = "interactWithControl({\"controlName\":\"Label1\",\"propertyName\":\"Text\"}, {\"Text\":1})"; + + Assert.Equal(expected, script); + } + + [Fact] + public void SetPropertyValueDoesNotExecuteInjectedPropertyName() + { + // Arrange - setPropertyValue routes object values through interactWithControl + var engine = CreateSdkEngine(forceScalarBranch: false); + + // Act + engine.Execute($"PowerAppsTestEngine.setPropertyValue({ItemPathJson(InjectionPropertyName)}, {{ Value: 1 }});"); + var script = engine.Evaluate("capturedScript").AsString(); + + InstallProbe(engine); + engine.Execute($"eval({JsonConvert.SerializeObject(script)});"); + + // Assert + Assert.False(engine.Evaluate("injected").AsBoolean()); + Assert.Equal(InjectionPropertyName, engine.Evaluate("probedKeys[0]").AsString()); + } + + [Fact] + public void SourceDoesNotInterpolatePropertyNameUnescaped() + { + // Arrange + var source = GetCanvasSdkSource(); + + // Assert - guards against reintroducing the hand written quoting of the property name + Assert.DoesNotContain("{\"${itemPath.propertyName}\"", source); + Assert.Contains("JSON.stringify(itemPath.propertyName)", source); + } + } +} diff --git a/src/testengine.provider.canvas.tests/testengine.provider.canvas.tests.csproj b/src/testengine.provider.canvas.tests/testengine.provider.canvas.tests.csproj index 63671d1b0..0657607e5 100644 --- a/src/testengine.provider.canvas.tests/testengine.provider.canvas.tests.csproj +++ b/src/testengine.provider.canvas.tests/testengine.provider.canvas.tests.csproj @@ -19,6 +19,7 @@ + @@ -37,4 +38,9 @@ + + + + + diff --git a/src/testengine.provider.canvas/JS/CanvasAppSdk.js b/src/testengine.provider.canvas/JS/CanvasAppSdk.js index f035d8d13..a0a0360b0 100644 --- a/src/testengine.provider.canvas/JS/CanvasAppSdk.js +++ b/src/testengine.provider.canvas/JS/CanvasAppSdk.js @@ -66,10 +66,10 @@ function interactWithControl(itemPath, value) { for (var index in values) { valuesJsonArr[`${index}`] = `${JSON.stringify(values[index])}`; } - var valueJson = `{"${itemPath.propertyName}":${valuesJsonArr}}`; + var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${valuesJsonArr}}`; script = `interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`; } else { - var valueJson = `{"${itemPath.propertyName}":${value}}`; + var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${value}}`; script = `interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`; } return executePublishedAppScript(script); diff --git a/src/testengine.provider.mda.tests/PowerAppsTestEngineMDACustomInjectionTests.cs b/src/testengine.provider.mda.tests/PowerAppsTestEngineMDACustomInjectionTests.cs new file mode 100644 index 000000000..8a49a1782 --- /dev/null +++ b/src/testengine.provider.mda.tests/PowerAppsTestEngineMDACustomInjectionTests.cs @@ -0,0 +1,210 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using Jint; +using Microsoft.PowerApps.TestEngine.Providers; +using Newtonsoft.Json; + +namespace Microsoft.PowerApps.TestEngine.Tests.PowerApps +{ + /// + /// Regression tests for the CWE-94 code injection fix in PowerAppsTestEngineMDACustom.js. + /// + /// PowerAppsModelDrivenCanvas.interactWithControl() builds a script string that is handed to + /// executePublishedAppScript(), which runs it through eval(). The property name comes from the + /// test plan (.fx.yaml), so it has to be escaped with JSON.stringify() instead of being wrapped + /// in hand written quotes, otherwise a crafted name can close the object literal and the + /// argument list and append arbitrary statements to the evaluated script. + /// + public class PowerAppsTestEngineMDACustomInjectionTests + { + private const string CustomResourceName = "testengine.provider.mda.PowerAppsTestEngineMDACustom.js"; + + /// + /// Property name that closes the generated object literal and the enclosing argument list, + /// appends a statement, then reopens both so the injected script still parses. + /// + private const string InjectionPropertyName = "a\":0});injected = true;({\"b"; + + private static string GetCustomScriptSource() + { + var assembly = typeof(ModelDrivenApplicationProvider).Assembly; + + using (var stream = assembly.GetManifestResourceStream(CustomResourceName)) + { + Assert.True(stream != null, $"Embedded resource {CustomResourceName} was not found"); + + using (var reader = new StreamReader(stream)) + { + return reader.ReadToEnd(); + } + } + } + + /// + /// Returns the source of the script building interactWithControl overload. + /// The class declares interactWithControl twice and the later in-app definition wins at + /// runtime, so the builder has to be evaluated on its own in order to be exercised. + /// + private static string ExtractScriptBuildingInteractWithControl(string source) + { + var start = source.IndexOf("static interactWithControl", StringComparison.Ordinal); + Assert.True(start >= 0, $"interactWithControl was not found in {CustomResourceName}"); + + var open = source.IndexOf('{', start); + Assert.True(open > start, "The body of interactWithControl could not be located"); + + var depth = 0; + for (var index = open; index < source.Length; index++) + { + if (source[index] == '{') + { + depth++; + } + else if (source[index] == '}') + { + depth--; + + if (depth == 0) + { + return source.Substring(start, index - start + 1); + } + } + } + + Assert.Fail("The body of interactWithControl is not brace balanced"); + return string.Empty; + } + + /// + /// Hosts the real builder next to a stubbed executePublishedAppScript so the generated + /// script can be inspected instead of evaluated straight away. + /// + /// Selects the array or the scalar branch of the builder. + private static Engine CreateBuilderEngine(bool treatValuesAsArray) + { + var builder = ExtractScriptBuildingInteractWithControl(GetCustomScriptSource()); + + var harness = @" +var capturedScript = null; +var injected = false; +function isArray(value) { return " + (treatValuesAsArray ? "true" : "false") + @"; } +class PowerAppsModelDrivenCanvas { + static executePublishedAppScript(scriptToExecute) { + capturedScript = scriptToExecute; + return scriptToExecute; + } + + " + builder + @" +}"; + + var engine = new Engine(); + engine.Execute(harness); + return engine; + } + + private static string BuildScript(Engine engine, string propertyName, string valueLiteral) + { + var itemPath = JsonConvert.SerializeObject(new { controlName = "TextInput1", propertyName }); + + engine.Execute($"PowerAppsModelDrivenCanvas.interactWithControl({itemPath}, {valueLiteral});"); + + return engine.Evaluate("capturedScript").AsString(); + } + + [Theory] + [InlineData(true, "{ Value: 1 }")] + [InlineData(false, "1")] + public void InteractWithControlEscapesPropertyNameInGeneratedScript(bool treatValuesAsArray, string valueLiteral) + { + // Arrange + var engine = CreateBuilderEngine(treatValuesAsArray); + + // Act + var script = BuildScript(engine, InjectionPropertyName, valueLiteral); + + // Assert - the quote that would terminate the key is escaped + Assert.Contains("a\\\":0});injected", script); + + // Assert - the payload stays inert when the generated script is evaluated + engine.Execute("eval(capturedScript);"); + Assert.False(engine.Evaluate("injected").AsBoolean()); + } + + [Theory] + [InlineData(true, "{ Value: 1 }")] + [InlineData(false, "1")] + public void InteractWithControlDeliversPropertyNameAsSingleKey(bool treatValuesAsArray, string valueLiteral) + { + // Arrange + var engine = CreateBuilderEngine(treatValuesAsArray); + var script = BuildScript(engine, InjectionPropertyName, valueLiteral); + + // Swap the builder for a probe so the evaluated script reports what it actually passes on + engine.Execute("var receivedKeys = null;"); + engine.Execute("PowerAppsModelDrivenCanvas.interactWithControl = function (itemPath, value) { receivedKeys = Object.keys(value); return true; };"); + + // Act + engine.Execute($"eval({JsonConvert.SerializeObject(script)});"); + + // Assert - the whole payload arrives as one inert key rather than as extra statements + Assert.False(engine.Evaluate("injected").AsBoolean()); + Assert.Equal(1, (int)engine.Evaluate("receivedKeys.length").AsNumber()); + Assert.Equal(InjectionPropertyName, engine.Evaluate("receivedKeys[0]").AsString()); + } + + [Theory] + [InlineData(true, "{ Value: 1 }", "{\"Text\":1}")] + [InlineData(false, "1", "{\"Text\":1}")] + public void InteractWithControlIsUnchangedForOrdinaryPropertyNames(bool treatValuesAsArray, string valueLiteral, string expectedValueJson) + { + // Arrange + var engine = CreateBuilderEngine(treatValuesAsArray); + + // Act + var script = BuildScript(engine, "Text", valueLiteral); + + // Assert - escaping the key must not alter the script produced for a normal plan + var expected = "PowerAppsModelDrivenCanvas.interactWithControl(" + + "{\"controlName\":\"TextInput1\",\"propertyName\":\"Text\"}, " + + expectedValueJson + + ")"; + + Assert.Equal(expected, script); + } + + [Fact] + public void SetPropertyValueDoesNotExecuteInjectedPropertyName() + { + // Arrange + var engine = new Engine(); + engine.Execute(Common.MockJavaScript( + "mockPageType = 'custom'; var injected = false", + "custom", + interfaceResourceNames: new List + { + "testengine.provider.mda.PowerAppsTestEngineMDA.js", + "testengine.provider.mda.PowerAppsTestEngineMDACustom.js" + })); + + var itemPath = JsonConvert.SerializeObject(new { controlName = "TextInput1", propertyName = InjectionPropertyName }); + + // Act + engine.Execute($"PowerAppsTestEngine.setPropertyValue({itemPath}, {{ Value: 1 }});"); + + // Assert + Assert.False(engine.Evaluate("injected").AsBoolean()); + } + + [Fact] + public void SourceDoesNotInterpolatePropertyNameUnescaped() + { + // Arrange + var source = GetCustomScriptSource(); + + // Assert - guards against reintroducing the hand written quoting of the property name + Assert.DoesNotContain("{\"${itemPath.propertyName}\"", source); + Assert.Contains("JSON.stringify(itemPath.propertyName)", source); + } + } +} diff --git a/src/testengine.provider.mda/PowerAppsTestEngineMDACustom.js b/src/testengine.provider.mda/PowerAppsTestEngineMDACustom.js index aa07faba5..264fd08e4 100644 --- a/src/testengine.provider.mda/PowerAppsTestEngineMDACustom.js +++ b/src/testengine.provider.mda/PowerAppsTestEngineMDACustom.js @@ -56,10 +56,10 @@ class PowerAppsModelDrivenCanvas { for (var index in values) { valuesJsonArr[`${index}`] = `${JSON.stringify(values[index])}`; } - var valueJson = `{"${itemPath.propertyName}":${valuesJsonArr}}`; + var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${valuesJsonArr}}`; script = `PowerAppsModelDrivenCanvas.interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`; } else { - var valueJson = `{"${itemPath.propertyName}":${value}}`; + var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${value}}`; script = `PowerAppsModelDrivenCanvas.interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`; } return PowerAppsModelDrivenCanvas.executePublishedAppScript(script);