Skip to content

Commit 74f2008

Browse files
claudiamurialdoclaudiamurialdo
andauthored
Fix code QL security issues (#1231)
* Fix code QL security issues: - Added a timeout to the Regex constructor to prevent potential Regular Expression Denial of Service (ReDoS). - Sanitized the user-provided filename in GetBinary. - Improved debug logging in GXMetadata.cs by logging class names instead of constructor arguments, and removed ConstructorArgsString to prevent unsanitized user input from being written to log files. * Add directory traversal checks for ZIP extraction --------- Co-authored-by: claudiamurialdo <[email protected]>
1 parent 0eb2a56 commit 74f2008

File tree

4 files changed

+20
-13
lines changed

4 files changed

+20
-13
lines changed

dotnet/src/dotnetcore/GxClasses.Web/Middleware/GXRouting.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ internal class GXRouting : IGXRouting
4141
public static bool AzureRuntime;
4242
public AzureDeployFeature AzureDeploy = new AzureDeployFeature();
4343
public static string AzureFunctionName;
44+
const int REGEX_DEFAULT_MATCH_TIMEOUT_SECONDS= 10;
4445

45-
static Regex SDSVC_PATTERN = new Regex("([^/]+/)*(sdsvc_[^/]+/[^/]+)(\\?.*)*");
46+
static Regex SDSVC_PATTERN = new Regex("([^/]+/)*(sdsvc_[^/]+/[^/]+)(\\?.*)*", RegexOptions.None, TimeSpan.FromSeconds(REGEX_DEFAULT_MATCH_TIMEOUT_SECONDS));
4647

4748
internal const string PRIVATE_DIR = "private";
4849
public Dictionary<string, string> servicesPathUrl = new Dictionary<string, string>();

dotnet/src/dotnetframework/GxClasses/Data/GXDataCommon.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ protected static byte[] GetBinary(string fileNameParm, bool dbBlob)
947947
}
948948
else
949949
{
950-
GXLogging.Error(log, "Not a valid URI: ", fileName);
950+
GXLogging.WarnSanitized(log, "Not a valid URI: ", fileName);
951951
throw new GxADODataException("GxCommand. Not a valid uri: " + fileName);
952952
}
953953
return binary;

dotnet/src/dotnetframework/GxClasses/Helpers/GXMetadata.cs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -226,19 +226,9 @@ static public object FindInstance(string defaultAssemblyName, string clss, Objec
226226
static public object FindInstance(string defaultAssemblyName, string nspace, string clss, Object[] constructorArgs, Assembly defaultAssembly, bool ignoreCase=false)
227227
{
228228
Type objType = FindType( defaultAssemblyName, nspace, clss, defaultAssembly, ignoreCase);
229-
GXLogging.Debug(log, "CreateInstance, Args ", ConstructorArgsString(constructorArgs));
229+
GXLogging.Debug(log, "CreateInstance class:", clss);
230230
return Activator.CreateInstance(objType, constructorArgs);
231231
}
232-
internal static string ConstructorArgsString(Object[] constructorArgs)
233-
{
234-
string argsConstr = "";
235-
for (int i = 0; constructorArgs != null && i < constructorArgs.Length; i++)
236-
{
237-
argsConstr += "'" + constructorArgs[i] + "' ";
238-
}
239-
return argsConstr;
240-
}
241-
242232
static public void ExecuteVoidRef(object o, string mthd, Object[] args)
243233
{
244234
try

dotnet/src/dotnetframework/GxCompress/GXCompressor.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,14 @@ private static void DecompressZip(FileInfo file, string outputPath)
603603
foreach (var entry in archive.Entries)
604604
{
605605
string fullPath = Path.Combine(outputPath, entry.FullName);
606+
string destFileName = Path.GetFullPath(fullPath);
607+
string fullDestDirPath = Path.GetFullPath(outputPath + Path.DirectorySeparatorChar);
608+
if (!destFileName.StartsWith(fullDestDirPath))
609+
{
610+
throw new InvalidOperationException("Entry is outside the target dir: " + destFileName);
611+
}
612+
613+
606614
if (string.IsNullOrEmpty(entry.Name))
607615
{
608616
Directory.CreateDirectory(fullPath);
@@ -742,6 +750,14 @@ private static void DecompressJar(FileInfo file, string outputPath)
742750
foreach (var entry in archive.Entries)
743751
{
744752
string destinationPath = Path.Combine(outputPath, entry.FullName);
753+
string destFileName = Path.GetFullPath(destinationPath);
754+
string fullDestDirPath = Path.GetFullPath(outputPath + Path.DirectorySeparatorChar);
755+
if (!destFileName.StartsWith(fullDestDirPath))
756+
{
757+
throw new InvalidOperationException("Entry is outside the target dir: " + destFileName);
758+
}
759+
760+
745761
if (string.IsNullOrEmpty(entry.Name))
746762
{
747763
Directory.CreateDirectory(destinationPath);

0 commit comments

Comments
 (0)