SQL injection via push notification configuration is possible (#1016)

* fix: use ef core to build query instead of raw sql * fix: make the methods async * chore: add debug output * ci: docker compose wait * chore: remove debug output
nmshd · Jan 11, 2025 · 1a0c326 · 1a0c326
1 parent 708ee89
commit 1a0c326
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 23 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -230,6 +230,7 @@ jobs:
         run: |
           docker compose -f ./.ci/compose.test.yml -f ./.ci/compose.test.${{matrix.database}}.yml down -v
           docker compose -f ./.ci/compose.test.yml -f ./.ci/compose.test.${{matrix.database}}.yml up --no-build --wait -d
+          docker compose -f ./.ci/compose.test.yml -f ./.ci/compose.test.${{matrix.database}}.yml wait admin-cli
 
       - name: Run integration tests
         run: dotnet test --no-restore --no-build --logger "GitHubActions;summary.includeNotFoundTests=false;summary.includeSkippedTests=false;summary.includePassedTests=false" ${{matrix.test-project.path}}

diff --git a/Modules/Devices/src/Devices.ConsumerApi/Controllers/IdentitiesController.cs b/Modules/Devices/src/Devices.ConsumerApi/Controllers/IdentitiesController.cs
@@ -19,6 +19,7 @@
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.Logging;
 using OpenIddict.Core;
 using OpenIddict.Validation.AspNetCore;
 
@@ -29,12 +30,15 @@ namespace Backbone.Modules.Devices.ConsumerApi.Controllers;
 public class IdentitiesController : ApiControllerBase
 {
     private readonly OpenIddictApplicationManager<CustomOpenIddictEntityFrameworkCoreApplication> _applicationManager;
+    private readonly ILogger<IdentitiesController> _logger;
 
     public IdentitiesController(
         IMediator mediator,
-        OpenIddictApplicationManager<CustomOpenIddictEntityFrameworkCoreApplication> applicationManager) : base(mediator)
+        OpenIddictApplicationManager<CustomOpenIddictEntityFrameworkCoreApplication> applicationManager,
+        ILogger<IdentitiesController> logger) : base(mediator)
     {
         _applicationManager = applicationManager;
+        _logger = logger;
     }
 
     [HttpPost]

diff --git a/Modules/Devices/src/Devices.ConsumerApi/DevicesModule.cs b/Modules/Devices/src/Devices.ConsumerApi/DevicesModule.cs
@@ -60,14 +60,14 @@ public override void PostStartupValidation(IServiceProvider serviceProvider)
         {
             var apnsOptions = serviceProvider.GetRequiredService<IOptions<ApnsOptions>>().Value;
             supportedApnsBundleIds = apnsOptions.GetSupportedBundleIds();
-            failingApnsBundleIds = devicesDbContext.GetApnsBundleIdsForWhichNoConfigurationExists(supportedApnsBundleIds);
+            failingApnsBundleIds = devicesDbContext.GetApnsBundleIdsForWhichNoConfigurationExists(supportedApnsBundleIds).GetAwaiter().GetResult();
         }
 
         if (configuration.Value.Infrastructure.PushNotifications.Providers.Fcm is { Enabled: true })
         {
             var fcmOptions = serviceProvider.GetRequiredService<IOptions<FcmOptions>>().Value;
             supportedFcmAppIds = fcmOptions.GetSupportedAppIds();
-            failingFcmAppIds = devicesDbContext.GetFcmAppIdsForWhichNoConfigurationExists(supportedFcmAppIds);
+            failingFcmAppIds = devicesDbContext.GetFcmAppIdsForWhichNoConfigurationExists(supportedFcmAppIds).GetAwaiter().GetResult();
         }
 
         if (failingFcmAppIds.Count + failingApnsBundleIds.Count > 0)

diff --git a/Modules/Devices/src/Devices.Infrastructure/Persistence/Database/DevicesDbContext.cs b/Modules/Devices/src/Devices.Infrastructure/Persistence/Database/DevicesDbContext.cs
@@ -137,14 +137,14 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
 #endif
     }
 
-    public List<string> GetFcmAppIdsForWhichNoConfigurationExists(ICollection<string> supportedAppIds)
+    public async Task<List<string>> GetFcmAppIdsForWhichNoConfigurationExists(ICollection<string> supportedAppIds)
     {
-        return GetAppIdsForWhichNoConfigurationExists("fcm", supportedAppIds);
+        return await GetAppIdsForWhichNoConfigurationExists("fcm", supportedAppIds);
     }
 
-    public List<string> GetApnsBundleIdsForWhichNoConfigurationExists(ICollection<string> supportedAppIds)
+    public async Task<List<string>> GetApnsBundleIdsForWhichNoConfigurationExists(ICollection<string> supportedAppIds)
     {
-        return GetAppIdsForWhichNoConfigurationExists("apns", supportedAppIds);
+        return await GetAppIdsForWhichNoConfigurationExists("apns", supportedAppIds);
     }
 
     public override int SaveChanges()
@@ -174,26 +174,15 @@ public override int SaveChanges(bool acceptAllChangesOnSuccess)
         return result;
     }
 
-    private List<string> GetAppIdsForWhichNoConfigurationExists(string platform, ICollection<string> supportedAppIds)
+    private async Task<List<string>> GetAppIdsForWhichNoConfigurationExists(string platform, ICollection<string> supportedAppIds)
     {
-        var query = PnsRegistrations.FromSqlRaw(
-            Database.IsNpgsql()
-                ? $"""
-                     SELECT "AppId"
-                     FROM "Devices"."PnsRegistrations"
-                     WHERE "Handle" LIKE '{platform}%'
-                   """
-                : $"""
-                     SELECT "AppId"
-                     FROM [Devices].[PnsRegistrations]
-                     WHERE Handle LIKE '{platform}%'
-                   """);
-
-        return query
+        return await PnsRegistrations
+            .AsNoTracking()
+            .Where(x => ((string)(object)x.Handle).StartsWith(platform))
             .Where(x => !supportedAppIds.Contains(x.AppId))
             .Select(x => x.AppId)
             .Distinct()
-            .ToList();
+            .ToListAsync();
     }
 
     protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)