Skip to content

Commit aaa183d

Browse files
brfloodCopilot
andcommitted
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>
1 parent da68e46 commit aaa183d

2 files changed

Lines changed: 212 additions & 2 deletions

File tree

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
using Jint;
5+
using Microsoft.PowerApps.TestEngine.Providers;
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 PowerAppsTestEngineMDACustom.js.
12+
///
13+
/// PowerAppsModelDrivenCanvas.interactWithControl() builds a script string that is handed to
14+
/// executePublishedAppScript(), which runs it through eval(). The property name comes from the
15+
/// test plan (.fx.yaml), so it has to be escaped with JSON.stringify() instead of being wrapped
16+
/// in hand written quotes, otherwise a crafted name can close the object literal and the
17+
/// argument list and append arbitrary statements to the evaluated script.
18+
/// </summary>
19+
public class PowerAppsTestEngineMDACustomInjectionTests
20+
{
21+
private const string CustomResourceName = "testengine.provider.mda.PowerAppsTestEngineMDACustom.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 GetCustomScriptSource()
30+
{
31+
var assembly = typeof(ModelDrivenApplicationProvider).Assembly;
32+
33+
using (var stream = assembly.GetManifestResourceStream(CustomResourceName))
34+
{
35+
Assert.True(stream != null, $"Embedded resource {CustomResourceName} was not found");
36+
37+
using (var reader = new StreamReader(stream))
38+
{
39+
return reader.ReadToEnd();
40+
}
41+
}
42+
}
43+
44+
/// <summary>
45+
/// Returns the source of the script building interactWithControl overload.
46+
/// The class declares interactWithControl twice and the later in-app definition wins at
47+
/// runtime, so the builder has to be evaluated on its own in order to be exercised.
48+
/// </summary>
49+
private static string ExtractScriptBuildingInteractWithControl(string source)
50+
{
51+
var start = source.IndexOf("static interactWithControl", StringComparison.Ordinal);
52+
Assert.True(start >= 0, $"interactWithControl was not found in {CustomResourceName}");
53+
54+
var open = source.IndexOf('{', start);
55+
Assert.True(open > start, "The body of interactWithControl could not be located");
56+
57+
var depth = 0;
58+
for (var index = open; index < source.Length; index++)
59+
{
60+
if (source[index] == '{')
61+
{
62+
depth++;
63+
}
64+
else if (source[index] == '}')
65+
{
66+
depth--;
67+
68+
if (depth == 0)
69+
{
70+
return source.Substring(start, index - start + 1);
71+
}
72+
}
73+
}
74+
75+
Assert.Fail("The body of interactWithControl is not brace balanced");
76+
return string.Empty;
77+
}
78+
79+
/// <summary>
80+
/// Hosts the real builder next to a stubbed executePublishedAppScript so the generated
81+
/// script can be inspected instead of evaluated straight away.
82+
/// </summary>
83+
/// <param name="treatValuesAsArray">Selects the array or the scalar branch of the builder.</param>
84+
private static Engine CreateBuilderEngine(bool treatValuesAsArray)
85+
{
86+
var builder = ExtractScriptBuildingInteractWithControl(GetCustomScriptSource());
87+
88+
var harness = @"
89+
var capturedScript = null;
90+
var injected = false;
91+
function isArray(value) { return " + (treatValuesAsArray ? "true" : "false") + @"; }
92+
class PowerAppsModelDrivenCanvas {
93+
static executePublishedAppScript(scriptToExecute) {
94+
capturedScript = scriptToExecute;
95+
return scriptToExecute;
96+
}
97+
98+
" + builder + @"
99+
}";
100+
101+
var engine = new Engine();
102+
engine.Execute(harness);
103+
return engine;
104+
}
105+
106+
private static string BuildScript(Engine engine, string propertyName, string valueLiteral)
107+
{
108+
var itemPath = JsonConvert.SerializeObject(new { controlName = "TextInput1", propertyName });
109+
110+
engine.Execute($"PowerAppsModelDrivenCanvas.interactWithControl({itemPath}, {valueLiteral});");
111+
112+
return engine.Evaluate("capturedScript").AsString();
113+
}
114+
115+
[Theory]
116+
[InlineData(true, "{ Value: 1 }")]
117+
[InlineData(false, "1")]
118+
public void InteractWithControlEscapesPropertyNameInGeneratedScript(bool treatValuesAsArray, string valueLiteral)
119+
{
120+
// Arrange
121+
var engine = CreateBuilderEngine(treatValuesAsArray);
122+
123+
// Act
124+
var script = BuildScript(engine, InjectionPropertyName, valueLiteral);
125+
126+
// Assert - the quote that would terminate the key is escaped
127+
Assert.Contains("a\\\":0});injected", script);
128+
129+
// Assert - the payload stays inert when the generated script is evaluated
130+
engine.Execute("eval(capturedScript);");
131+
Assert.False(engine.Evaluate("injected").AsBoolean());
132+
}
133+
134+
[Theory]
135+
[InlineData(true, "{ Value: 1 }")]
136+
[InlineData(false, "1")]
137+
public void InteractWithControlDeliversPropertyNameAsSingleKey(bool treatValuesAsArray, string valueLiteral)
138+
{
139+
// Arrange
140+
var engine = CreateBuilderEngine(treatValuesAsArray);
141+
var script = BuildScript(engine, InjectionPropertyName, valueLiteral);
142+
143+
// Swap the builder for a probe so the evaluated script reports what it actually passes on
144+
engine.Execute("var receivedKeys = null;");
145+
engine.Execute("PowerAppsModelDrivenCanvas.interactWithControl = function (itemPath, value) { receivedKeys = Object.keys(value); return true; };");
146+
147+
// Act
148+
engine.Execute($"eval({JsonConvert.SerializeObject(script)});");
149+
150+
// Assert - the whole payload arrives as one inert key rather than as extra statements
151+
Assert.False(engine.Evaluate("injected").AsBoolean());
152+
Assert.Equal(1, (int)engine.Evaluate("receivedKeys.length").AsNumber());
153+
Assert.Equal(InjectionPropertyName, engine.Evaluate("receivedKeys[0]").AsString());
154+
}
155+
156+
[Theory]
157+
[InlineData(true, "{ Value: 1 }", "{\"Text\":1}")]
158+
[InlineData(false, "1", "{\"Text\":1}")]
159+
public void InteractWithControlIsUnchangedForOrdinaryPropertyNames(bool treatValuesAsArray, string valueLiteral, string expectedValueJson)
160+
{
161+
// Arrange
162+
var engine = CreateBuilderEngine(treatValuesAsArray);
163+
164+
// Act
165+
var script = BuildScript(engine, "Text", valueLiteral);
166+
167+
// Assert - escaping the key must not alter the script produced for a normal plan
168+
var expected = "PowerAppsModelDrivenCanvas.interactWithControl("
169+
+ "{\"controlName\":\"TextInput1\",\"propertyName\":\"Text\"}, "
170+
+ expectedValueJson
171+
+ ")";
172+
173+
Assert.Equal(expected, script);
174+
}
175+
176+
[Fact]
177+
public void SetPropertyValueDoesNotExecuteInjectedPropertyName()
178+
{
179+
// Arrange
180+
var engine = new Engine();
181+
engine.Execute(Common.MockJavaScript(
182+
"mockPageType = 'custom'; var injected = false",
183+
"custom",
184+
interfaceResourceNames: new List<string>
185+
{
186+
"testengine.provider.mda.PowerAppsTestEngineMDA.js",
187+
"testengine.provider.mda.PowerAppsTestEngineMDACustom.js"
188+
}));
189+
190+
var itemPath = JsonConvert.SerializeObject(new { controlName = "TextInput1", propertyName = InjectionPropertyName });
191+
192+
// Act
193+
engine.Execute($"PowerAppsTestEngine.setPropertyValue({itemPath}, {{ Value: 1 }});");
194+
195+
// Assert
196+
Assert.False(engine.Evaluate("injected").AsBoolean());
197+
}
198+
199+
[Fact]
200+
public void SourceDoesNotInterpolatePropertyNameUnescaped()
201+
{
202+
// Arrange
203+
var source = GetCustomScriptSource();
204+
205+
// Assert - guards against reintroducing the hand written quoting of the property name
206+
Assert.DoesNotContain("{\"${itemPath.propertyName}\"", source);
207+
Assert.Contains("JSON.stringify(itemPath.propertyName)", source);
208+
}
209+
}
210+
}

‎src/testengine.provider.mda/PowerAppsTestEngineMDACustom.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ class PowerAppsModelDrivenCanvas {
5656
for (var index in values) {
5757
valuesJsonArr[`${index}`] = `${JSON.stringify(values[index])}`;
5858
}
59-
var valueJson = `{"${itemPath.propertyName}":${valuesJsonArr}}`;
59+
var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${valuesJsonArr}}`;
6060
script = `PowerAppsModelDrivenCanvas.interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`;
6161
} else {
62-
var valueJson = `{"${itemPath.propertyName}":${value}}`;
62+
var valueJson = `{${JSON.stringify(itemPath.propertyName)}:${value}}`;
6363
script = `PowerAppsModelDrivenCanvas.interactWithControl(${JSON.stringify(itemPath)}, ${valueJson})`;
6464
}
6565
return PowerAppsModelDrivenCanvas.executePublishedAppScript(script);

0 commit comments

Comments
 (0)