Sanitize user input

docker-compose.yml

@@ -1,5 +1,3 @@
-version: "3.4"
-
 services:
   stac-browser:
     image: ghcr.io/geowerkstatt/stac-browser:latest

src/Geopilot.Api/Controllers/MandateController.cs

@@ -42,9 +42,9 @@ public MandateController(ILogger<MandateController> logger, Context context, IVa
     [SwaggerResponse(StatusCodes.Status200OK, "Returns list of mandates associated to the current user matching the optional filter criteria.", typeof(IEnumerable<DeliveryMandate>), new[] { "application/json" })]
     public async Task<IActionResult> Get(
         [FromQuery, SwaggerParameter("Filter mandates matching validation job file extension.")]
-        string jobId = "")
+        Guid jobId = default)
     {
-        logger.LogInformation("Getting mandates for job with id <{JobId}>.", jobId.ReplaceLineEndings(string.Empty));
+        logger.LogInformation("Getting mandates for job with id <{JobId}>.", jobId);
 
         var user = await context.GetUserByPrincipalAsync(User);
         if (user == null)
@@ -53,24 +53,24 @@ public async Task<IActionResult> Get(
         var mandates = context.DeliveryMandates
             .Where(m => m.Organisations.SelectMany(o => o.Users).Any(u => u.Id == user.Id));
 
-        if (Guid.TryParse(jobId, out var guid))
+        if (jobId != default)
         {
-            var job = validationService.GetJob(guid);
+            var job = validationService.GetJob(jobId);
             if (job is null)
             {
-                logger.LogTrace("Validation job with id <{JobId}> was not found.", guid.ToString());
+                logger.LogTrace("Validation job with id <{JobId}> was not found.", jobId);
                 return Ok(Array.Empty<DeliveryMandate>());
             }
 
-            logger.LogTrace("Filtering mandates for job with id <{JobId}>", guid.ToString());
+            logger.LogTrace("Filtering mandates for job with id <{JobId}>", jobId);
             var extension = Path.GetExtension(job.OriginalFileName);
             mandates = mandates
                 .Where(m => m.FileTypes.Contains(".*") || m.FileTypes.Contains(extension));
         }
 
         var result = await mandates.ToListAsync();
 
-        logger.LogInformation("Getting mandates with for job with id <{JobId}> resulted in <{MatchingMandatesCount}> matching mandates.", guid.ToString(), result.Count);
+        logger.LogInformation("Getting mandates with for job with id <{JobId}> resulted in <{MatchingMandatesCount}> matching mandates.", jobId, result.Count);
         return Ok(mandates);
     }
 }

src/Geopilot.Api/Controllers/ValidationController.cs

@@ -8,6 +8,7 @@
 using Swashbuckle.AspNetCore.Annotations;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
+using System.Web;
 
 namespace Geopilot.Api.Controllers;
 
@@ -166,9 +167,7 @@ public IActionResult GetStatus(Guid jobId)
     [SwaggerResponse(StatusCodes.Status404NotFound, "The job or log file cannot be found.", typeof(ProblemDetails), new[] { "application/json" })]
     public IActionResult Download(Guid jobId, string file)
     {
-        var sanitizedFilename = Path.GetFileName(file.Trim().ReplaceLineEndings(string.Empty));
-
-        logger.LogInformation("Download file <{File}> for job <{JobId}> requested.", sanitizedFilename, jobId.ToString());
+        logger.LogInformation("Download file <{File}> for job <{JobId}> requested.", HttpUtility.HtmlEncode(file), jobId);
         fileProvider.Initialize(jobId);
 
         var validationJob = validationService.GetJob(jobId);
@@ -178,15 +177,15 @@ public IActionResult Download(Guid jobId, string file)
             return Problem($"No job information available for job id <{jobId}>", statusCode: StatusCodes.Status404NotFound);
         }
 
-        if (!fileProvider.Exists(sanitizedFilename))
+        if (!fileProvider.Exists(file))
         {
-            logger.LogTrace("No log file <{File}> found for job id <{JobId}>", sanitizedFilename, jobId);
-            return Problem($"No log file <{sanitizedFilename}> found for job id <{jobId}>", statusCode: StatusCodes.Status404NotFound);
+            logger.LogTrace("No log file <{File}> found for job id <{JobId}>", HttpUtility.HtmlEncode(file), jobId);
+            return Problem($"No log file <{file}> found for job id <{jobId}>", statusCode: StatusCodes.Status404NotFound);
         }
 
-        var logFile = fileProvider.Open(sanitizedFilename);
-        var contentType = contentTypeProvider.GetContentTypeAsString(sanitizedFilename);
-        var logFileName = Path.GetFileNameWithoutExtension(validationJob.OriginalFileName) + "_log" + Path.GetExtension(sanitizedFilename);
+        var logFile = fileProvider.Open(file);
+        var contentType = contentTypeProvider.GetContentTypeAsString(file);
+        var logFileName = Path.GetFileNameWithoutExtension(validationJob.OriginalFileName) + "_log" + Path.GetExtension(file);
         return File(logFile, contentType, logFileName);
     }
 }

src/Geopilot.Api/Extensions.cs

@@ -0,0 +1,26 @@
+namespace Geopilot.Api;
+
+/// <summary>
+/// GeoPilot API extensions.
+/// </summary>
+public static class Extensions
+{
+    /// <summary>
+    /// Sanitizes a file name by removing invalid characters.
+    /// </summary>
+    /// <param name="fileName">The file name to sanitize.</param>
+    /// <returns>The sanitized file name.</returns>
+    public static string SanitizeFileName(this string fileName)
+    {
+        // Get invalid characters for file names and add some platform-specific ones.
+        var invalidFileNameChars = Path.GetInvalidFileNameChars()
+            .Concat(new[] { '?', '$', '*', '|', '<', '>', '"', ':' }).ToArray();
+
+        return new string(fileName
+            .Trim()
+            .ReplaceLineEndings(string.Empty)
+            .Replace("..", string.Empty)
+            .Replace("./", string.Empty)
+            .Where(x => !invalidFileNameChars.Contains(x)).ToArray());
+    }
+}

src/Geopilot.Api/FileAccess/PhysicalFileProvider.cs

@@ -44,7 +44,7 @@
     /// <inheritdoc/>
     public bool Exists(string file)
     {
-        return File.Exists(Path.Combine(HomeDirectory.FullName, file));
+        return File.Exists(Path.Combine(HomeDirectory.FullName, Extensions.SanitizeFileName(file)));
     }
 
     /// <inheritdoc/>

tests/Geopilot.Api.Test/Controllers/MandateControllerTest.cs

@@ -70,7 +70,7 @@ public async Task GetWithJobIdIncludesMatchingMandates()
                 .Setup(m => m.GetJob(jobId))
                 .Returns(new ValidationJob(jobId, "Original.xtf", "tmp.xtf"));
 
-            var result = (await mandateController.Get(jobId.ToString())) as OkObjectResult;
+            var result = (await mandateController.Get(jobId)) as OkObjectResult;
             var mandates = (result?.Value as IEnumerable<DeliveryMandate>)?.ToList();
 
             Assert.IsNotNull(mandates);
@@ -88,7 +88,7 @@ public async Task GetWithJobIdExcludesNonMatchinMandates()
                 .Setup(m => m.GetJob(jobId))
                 .Returns(new ValidationJob(jobId, "Original.csv", "tmp.csv"));
 
-            var result = (await mandateController.Get(jobId: jobId.ToString())) as OkObjectResult;
+            var result = (await mandateController.Get(jobId)) as OkObjectResult;
             var mandates = (result?.Value as IEnumerable<DeliveryMandate>)?.ToList();
 
             Assert.IsNotNull(mandates);
@@ -106,7 +106,7 @@ public async Task GetWithInvalidJobIdReturnsEmptyArray()
                 .Setup(m => m.GetJob(jobId))
                 .Returns(() => null);
 
-            var result = (await mandateController.Get(jobId: jobId.ToString())) as OkObjectResult;
+            var result = (await mandateController.Get(jobId)) as OkObjectResult;
             var mandates = (result?.Value as IEnumerable<DeliveryMandate>)?.ToList();
 
             Assert.IsNotNull(mandates);

tests/Geopilot.Api.Test/Extensions.cs

@@ -0,0 +1,40 @@
+namespace Geopilot.Api.Controllers;
+
+[TestClass]
+public class Extensions
+{
+    [TestMethod]
+    public void SanitizeFileNameForValidFileNames()
+    {
+        AssertSanitizeFileName("SQUIRRELGENESIS", "SQUIRRELGENESIS");
+        AssertSanitizeFileName("JUNIORARK.xyz", "JUNIORARK.xyz");
+        AssertSanitizeFileName("PEEVEDBEAM-ANT.MESS.abc", "PEEVEDBEAM-ANT.MESS.abc");
+        AssertSanitizeFileName("WEIRD WATER.example", "WEIRD WATER.example");
+        AssertSanitizeFileName("AUTOFIRE123.doc", "AUTOFIRE123.doc");
+        AssertSanitizeFileName("SUNNY(1).doc", "SUNNY(1).doc");
+        AssertSanitizeFileName("ODD_MONKEY.doc", "ODD_MONKEY.doc");
+        AssertSanitizeFileName("SILLY,MONKEY.docx", "SILLY,MONKEY.docx");
+        AssertSanitizeFileName("CamelCase.bat", "CamelCase.bat");
+        AssertSanitizeFileName("SLICKER-CHIPMUNK.bat", "SLICKER-CHIPMUNK.bat");
+    }
+
+    [TestMethod]
+    public void SanitizeFileNameForInvalidFileNames()
+    {
+        AssertSanitizeFileName("CHIPMUNKWALK", "  CHIPMUNKWALK  ");
+        AssertSanitizeFileName("SLEEPYBOUNCE", "SLEEPYBOUNCE\n");
+        AssertSanitizeFileName("PLOWARK", "PLOWARK\r");
+        AssertSanitizeFileName("JUNIORGLEE", "JUNIORGLEE\t");
+        AssertSanitizeFileName("SILLYWATER", "SILLYWATER\r\n");
+        AssertSanitizeFileName("LATENTROUTE34", "LATENTROUTE?34");
+        AssertSanitizeFileName("TRAWLSOUFFLE", "/TRAWLSOUFFLE*");
+        AssertSanitizeFileName("VIOLENTIRON", "><VIOLENTIRON\"|");
+        AssertSanitizeFileName("YELLOWBAGEL", "YELLOWBAGEL://");
+        AssertSanitizeFileName("ZANYWATER", "ZANYWATER$");
+        AssertSanitizeFileName("SLICKERCANDID", "..\\SLICKERCANDID");
+        AssertSanitizeFileName("DIREFOOT", "./DIREFOOT:");
+    }
+
+    private static void AssertSanitizeFileName(string expected, string fileName)
+        => Assert.AreEqual(expected, fileName.SanitizeFileName());
+}