Skip to content

Commit 2c0342d

Browse files
brfloodCopilot
brflood
and
Copilot
authored
Fix CWE-94 code injection in MDA and canvas interactWithControl (#688)
* Fix CWE-94 code injection in MDA interactWithControl interactWithControl() built the value object literal by wrapping itemPath.propertyName in hand written quotes: var valueJson = `{"${itemPath.propertyName}":${value}}`; That string is concatenated into a script which executePublishedAppScript() runs through eval(). The property name originates from the test plan (.fx.yaml), so a crafted name such as `a":0});payload();({"b` closes the object literal and the argument list and appends arbitrary statements to the evaluated script. Escape the key with JSON.stringify() in both the array and the scalar branch. Ordinary property names serialize identically, so the generated script is unchanged for existing test plans. Adds PowerAppsTestEngineMDACustomInjectionTests covering the escaping, the inertness of the payload once the generated script is evaluated, and the unchanged output for ordinary property names. The class declares interactWithControl twice and the later definition wins at runtime, so the tests extract the script building overload from the embedded resource and evaluate it in isolation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix the same CWE-94 code injection in CanvasAppSdk.js interactWithControl() built the value object literal by wrapping itemPath.propertyName in hand written quotes, and executePublishedAppScript() marshals the resulting string into the published app where it is evaluated. A crafted property name from the test plan closes the object literal and the argument list and appends arbitrary statements to the script that crosses into the app. Unlike the model driven provider, these are plain top level functions with a single definition, so the path is reachable through PowerAppsTestEngine.setPropertyValue. Escape the key with JSON.stringify() in both the array and the scalar branch. Ordinary property names serialize identically, so the generated script is unchanged for existing test plans. Adds CanvasAppSdkInjectionTests, which loads the shipped script, stubs executePublishedAppScript to capture what would be sent to the app, and asserts the payload arrives as one inert key. The script is linked into the test project as an embedded resource so the tests run against the shipped file rather than a copy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent da68e46 commit 2c0342d

5 files changed

Lines changed: 398 additions & 4 deletions

File tree

‎src/testengine.provider.canvas.tests/CanvasAppSdkInjectionTests.cs‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
using System.Reflection;
5+
using Jint;
6+
using Newtonsoft.Json;
7+
8+
namespace Microsoft.PowerApps.TestEngine.Tests.PowerApps
9+
{
10+
/// <summary>
11+
/// Regression tests for the CWE-94 code injection fix in CanvasAppSdk.js.
12+
///
13+
/// interactWithControl() builds a script string that executePublishedAppScript() marshals into
14+
/// the published app and evaluates there. The property name comes from the test plan
15+
/// (.fx.yaml), so it has to be escaped with JSON.stringify() instead of being wrapped in hand
16+
/// written quotes, otherwise a crafted name can close the object literal and the argument list
17+
/// and append arbitrary statements to the script that crosses into the app.
18+
/// </summary>
19+
public class CanvasAppSdkInjectionTests
20+
{
21+
private const string SdkResourceName = "testengine.provider.canvas.tests.CanvasAppSdk.js";
22+
23+
/// <summary>
24+
/// Property name that closes the generated object literal and the enclosing argument list,
25+
/// appends a statement, then reopens both so the injected script still parses.
26+
/// </summary>
27+
private const string InjectionPropertyName = "a\":0});injected = true;({\"b";
28+
29+
private static string GetCanvasSdkSource()
30+
{
31+
var assembly = Assembly.GetExecutingAssembly();
32+
33+
using (var stream = assembly.GetManifestResourceStream(SdkResourceName))
34+
{
35+
Assert.True(stream != null, $"Embedded resource {SdkResourceName} was not found");
36+
37+
using (var reader = new StreamReader(stream))
38+
{
39+
return reader.ReadToEnd();
40+
}
41+
}
42+
}
43+
44+
/// <summary>
45+
/// Loads the shipped SDK and replaces executePublishedAppScript so the script that would be
46+
/// sent to the published app can be inspected instead of dispatched.
47+
/// </summary>
48+
/// <param name="forceScalarBranch">
49+
/// Object.values() always returns an array, so the scalar branch of interactWithControl is
50+
/// only reachable when isArray is stubbed out.
51+
/// </param>
52+
private static Engine CreateSdkEngine(bool forceScalarBranch)
53+
{
54+
var engine = new Engine();
55+
56+
// debugInfo is evaluated while the SDK loads and reads the published app telemetry
57+
engine.Execute("var Core = { Telemetry: {} };");
58+
engine.Execute("var capturedScript = null; var injected = false; var probedKeys = null;");
59+
60+
engine.Execute(GetCanvasSdkSource());
61+
62+
engine.Execute("executePublishedAppScript = function (scriptToExecute) { capturedScript = scriptToExecute; return scriptToExecute; };");
63+
64+
if (forceScalarBranch)
65+
{
66+
engine.Execute("isArray = function () { return false; };");
67+
}
68+
69+
return engine;
70+
}
71+
72+
private static string ItemPathJson(string propertyName)
73+
{
74+
return JsonConvert.SerializeObject(new { controlName = "Label1", propertyName });
75+
}
76+
77+
private static string BuildScript(Engine engine, string propertyName, string valueLiteral)
78+
{
79+
engine.Execute($"interactWithControl({ItemPathJson(propertyName)}, {valueLiteral});");
80+
81+
return engine.Evaluate("capturedScript").AsString();
82+
}
83+
84+
/// <summary>
85+
/// Stands in for the published app side implementation so the captured script can be run.
86+
/// </summary>
87+
private static void InstallProbe(Engine engine)
88+
{
89+
engine.Execute("interactWithControl = function (itemPath, value) { probedKeys = Object.keys(value); return true; };");
90+
}
91+
92+
[Theory]
93+
[InlineData(true, "{ Value: 1 }")]
94+
[InlineData(false, "1")]
95+
public void InteractWithControlEscapesPropertyNameInGeneratedScript(bool useArrayBranch, string valueLiteral)
96+
{
97+
// Arrange
98+
var engine = CreateSdkEngine(forceScalarBranch: !useArrayBranch);
99+
100+
// Act
101+
var script = BuildScript(engine, InjectionPropertyName, valueLiteral);
102+
103+
// Assert - the quote that would terminate the key is escaped
104+
Assert.Contains("a\\\":0});injected", script);
105+
106+
// Assert - the payload stays inert when the script reaches the published app
107+
InstallProbe(engine);
108+
engine.Execute($"eval({JsonConvert.SerializeObject(script)});");
109+
Assert.False(engine.Evaluate("injected").AsBoolean());
110+
}
111+
112+
[Theory]
113+
[InlineData(true, "{ Value: 1 }")]
114+
[InlineData(false, "1")]
115+
public void InteractWithControlDeliversPropertyNameAsSingleKey(bool useArrayBranch, string valueLiteral)
116+
{
117+
// Arrange
118+
var engine = CreateSdkEngine(forceScalarBranch: !useArrayBranch);
119+
var script = BuildScript(engine, InjectionPropertyName, valueLiteral);
120+
121+
InstallProbe(engine);
122+
123+
// Act
124+
engine.Execute($"eval({JsonConvert.SerializeObject(script)});");
125+
126+
// Assert - the whole payload arrives as one inert key rather than as extra statements
127+
Assert.False(engine.Evaluate("injected").AsBoolean());
128+
Assert.Equal(1, (int)engine.Evaluate("probedKeys.length").AsNumber());
129+
Assert.Equal(InjectionPropertyName, engine.Evaluate("probedKeys[0]").AsString());
130+
}
131+
132+
[Theory]
133+
[InlineData(true, "{ Value: 1 }")]
134+
[InlineData(false, "1")]
135+
public void InteractWithControlIsUnchangedForOrdinaryPropertyNames(bool useArrayBranch, string valueLiteral)
136+
{
137+
// Arrange
138+
var engine = CreateSdkEngine(forceScalarBranch: !useArrayBranch);
139+
140+
// Act
141+
var script = BuildScript(engine, "Text", valueLiteral);
142+
143+
// Assert - escaping the key must not alter the script produced for a normal plan
144+
var expected = "interactWithControl({\"controlName\":\"Label1\",\"propertyName\":\"Text\"}, {\"Text\":1})";
145+
146+
Assert.Equal(expected, script);
147+
}
148+
149+
[Fact]
150+
public void SetPropertyValueDoesNotExecuteInjectedPropertyName()
151+
{
152+
// Arrange - setPropertyValue routes object values through interactWithControl
153+
var engine = CreateSdkEngine(forceScalarBranch: false);
154+
155+
// Act
156+
engine.Execute($"PowerAppsTestEngine.setPropertyValue({ItemPathJson(InjectionPropertyName)}, {{ Value: 1 }});");
157+
var script = engine.Evaluate("capturedScript").AsString();
158+
159+
InstallProbe(engine);
160+
engine.Execute($"eval({JsonConvert.SerializeObject(script)});");
161+
162+
// Assert
163+
Assert.False(engine.Evaluate("injected").AsBoolean());
164+
Assert.Equal(InjectionPropertyName, engine.Evaluate("probedKeys[0]").AsString());
165+
}
166+
167+
[Fact]
168+
public void SourceDoesNotInterpolatePropertyNameUnescaped()
169+
{
170+
// Arrange
171+
var source = GetCanvasSdkSource();
172+
173+
// Assert - guards against reintroducing the hand written quoting of the property name
174+
Assert.DoesNotContain("{\"${itemPath.propertyName}\"", source);
175+
Assert.Contains("JSON.stringify(itemPath.propertyName)", source);
176+
}
177+
}
178+
}

‎src/testengine.provider.canvas.tests/testengine.provider.canvas.tests.csproj‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
</PropertyGroup>
2020

2121
<ItemGroup>
22+
<PackageReference Include="Jint" Version="4.1.0" />
2223
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
2324
<PackageReference Include="Moq" Version="4.20.72" />
2425
<PackageReference Include="xunit" Version="2.9.2" />
@@ -37,4 +38,9 @@
3738
<ProjectReference Include="..\testengine.provider.canvas\testengine.provider.canvas.csproj" />
3839
</ItemGroup>
3940

41+
<ItemGroup>
42+
<!-- Linked so the injection tests assert against the shipped script rather than a copy -->
43+
<EmbeddedResource Include="..\testengine.provider.canvas\JS\CanvasAppSdk.js" Link="CanvasAppSdk.js" LogicalName="testengine.provider.canvas.tests.CanvasAppSdk.js" />
44+
</ItemGroup>
45+
4046
</Project>

‎src/testengine.provider.canvas/JS/CanvasAppSdk.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ function interactWithControl(itemPath, value) {
6666
for (var index in values) {
6767
valuesJsonArr[`${index}`] = `${JSON.stringify(values[index])}`;
6868
}
69-
var valueJson = `{"${itemPath.propertyName}":${valuesJsonArr}}`;
69+
var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${valuesJsonArr}}`;
7070
script = `interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`;
7171
} else {
72-
var valueJson = `{"${itemPath.propertyName}":${value}}`;
72+
var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${value}}`;
7373
script = `interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`;
7474
}
7575
return executePublishedAppScript(script);

0 commit comments

Comments
 (0)