Fixed: Security Vulnerabilities allowing authentication to be bypass

src/Lidarr.Http/Extensions/Pipelines/CacheHeaderPipeline.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using Nancy;
 using Nancy.Bootstrapper;
 using Lidarr.Http.Frontend;
@@ -23,6 +23,8 @@ namespace Lidarr.Http.Extensions.Pipelines
 
         private void Handle(NancyContext context)
         {
+            if (context.Request.Method == "OPTIONS") return;
+
             if (_cacheableSpecification.IsCacheable(context))
             {
                 context.Response.Headers.EnableCache();
@@ -33,4 +35,4 @@ namespace Lidarr.Http.Extensions.Pipelines
             }
         }
     }
-}
+}

src/Lidarr.Http/Extensions/Pipelines/CorsPipeline.cs

@@ -1,7 +1,8 @@
-using System;
+using System;
 using System.Linq;
 using Nancy;
 using Nancy.Bootstrapper;
+using NzbDrone.Common.Extensions;
 
 namespace Lidarr.Http.Extensions.Pipelines
 {
@@ -11,10 +12,25 @@ namespace Lidarr.Http.Extensions.Pipelines
 
         public void Register(IPipelines pipelines)
         {
-            pipelines.AfterRequest.AddItemToEndOfPipeline((Action<NancyContext>) Handle);
+            pipelines.BeforeRequest.AddItemToEndOfPipeline(HandleRequest);
+            pipelines.AfterRequest.AddItemToEndOfPipeline(HandleResponse);
         }
 
-        private void Handle(NancyContext context)
+        private Response HandleRequest(NancyContext context)
+        {
+            if (context == null || context.Request.Method != "OPTIONS")
+            {
+                return null;
+            }
+
+            var response = new Response()
+                .WithStatusCode(HttpStatusCode.OK)
+                .WithContentType("");
+            ApplyResponseHeaders(response, context.Request);
+            return response;
+        }
+
+        private void HandleResponse(NancyContext context)
         {
             if (context == null || context.Response.Headers.ContainsKey(AccessControlHeaders.AllowOrigin))
             {
@@ -26,21 +42,39 @@ namespace Lidarr.Http.Extensions.Pipelines
 
         private static void ApplyResponseHeaders(Response response, Request request)
         {
-            var allowedMethods = "GET, OPTIONS, PATCH, POST, PUT, DELETE";
+            if (request.IsApiRequest())
-
-            if (response.Headers.ContainsKey("Allow"))
             {
-                allowedMethods = response.Headers["Allow"];
+                // Allow Cross-Origin access to the API since it's protected with the apikey, and nothing else.
+                ApplyCorsResponseHeaders(response, request, "*", "GET, OPTIONS, PATCH, POST, PUT, DELETE");
             }
-            
+            else if (request.IsSharedContentRequest())
-            var requestedHeaders = string.Join(", ", request.Headers[AccessControlHeaders.RequestHeaders]);
-
-            response.Headers.Add(AccessControlHeaders.AllowOrigin, "*");
-            response.Headers.Add(AccessControlHeaders.AllowMethods, allowedMethods);
-
-            if (request.Headers[AccessControlHeaders.RequestHeaders].Any())
             {
-                response.Headers.Add(AccessControlHeaders.AllowHeaders, requestedHeaders);
+                // Allow Cross-Origin access to specific shared content such as mediacovers and images.
+                ApplyCorsResponseHeaders(response, request, "*", "GET, OPTIONS");
+            }
+
+            // Disallow Cross-Origin access for any other route.
+        }
+
+        private static void ApplyCorsResponseHeaders(Response response, Request request, string allowOrigin, string allowedMethods)
+        {
+            response.Headers.Add(AccessControlHeaders.AllowOrigin, allowOrigin);
+
+            if (request.Method == "OPTIONS")
+            {
+                if (response.Headers.ContainsKey("Allow"))
+                {
+                    allowedMethods = response.Headers["Allow"];
+                }
+
+                response.Headers.Add(AccessControlHeaders.AllowMethods, allowedMethods);
+
+                if (request.Headers[AccessControlHeaders.RequestHeaders].Any())
+                {
+                    var requestedHeaders = request.Headers[AccessControlHeaders.RequestHeaders].Join(", ");
+
+                    response.Headers.Add(AccessControlHeaders.AllowHeaders, requestedHeaders);
+                }
             }
         }
     }

src/Lidarr.Http/Extensions/Pipelines/GZipPipeline.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.IO;
 using System.IO.Compression;
 using System.Linq;
@@ -33,7 +33,8 @@ namespace Lidarr.Http.Extensions.Pipelines
             try
             {
                 if (
-                   !response.ContentType.Contains("image")
+                   response.Contents != Response.NoBody
+                && !response.ContentType.Contains("image")
                 && !response.ContentType.Contains("font")
                 && request.Headers.AcceptEncoding.Any(x => x.Contains("gzip"))
                 && !AlreadyGzipEncoded(response)
@@ -84,4 +85,4 @@ namespace Lidarr.Http.Extensions.Pipelines
             return false;
         }
     }
-}
+}

src/Lidarr.Http/Extensions/RequestExtensions.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using Nancy;
 
 namespace Lidarr.Http.Extensions
@@ -48,5 +48,11 @@ namespace Lidarr.Http.Extensions
 
             return defaultValue;
         }
+
+        public static bool IsSharedContentRequest(this Request request)
+        {
+            return request.Path.StartsWith("/MediaCover/", StringComparison.InvariantCultureIgnoreCase) ||
+                   request.Path.StartsWith("/Content/Images/", StringComparison.InvariantCultureIgnoreCase);
+        }
     }
 }

src/Lidarr.Http/Frontend/Mappers/StaticResourceMapper.cs

@@ -1,4 +1,4 @@
-using System.IO;
+using System.IO;
 using NLog;
 using NzbDrone.Common.Disk;
 using NzbDrone.Common.EnvironmentInfo;
@@ -28,13 +28,15 @@ namespace Lidarr.Http.Frontend.Mappers
 
         public override bool CanHandle(string resourceUrl)
         {
-            if (resourceUrl.StartsWith("/Content/Images/Icons/manifest") ||
+            resourceUrl = resourceUrl.ToLowerInvariant();
-                resourceUrl.StartsWith("/Content/Images/Icons/browserconfig"))
+
+            if (resourceUrl.StartsWith("/content/images/icons/manifest") ||
+                resourceUrl.StartsWith("/content/images/icons/browserconfig"))
             {
                 return false;
             }
 
-            return resourceUrl.StartsWith("/Content") ||
+            return resourceUrl.StartsWith("/content") ||
                    resourceUrl.EndsWith(".js") ||
                    resourceUrl.EndsWith(".map") ||
                    resourceUrl.EndsWith(".css") ||
@@ -43,4 +45,4 @@ namespace Lidarr.Http.Frontend.Mappers
                    resourceUrl.EndsWith("oauth.html");
         }
     }
-}
+}

src/NzbDrone.Integration.Test/CorsFixture.cs

@@ -8,19 +8,26 @@ namespace NzbDrone.Integration.Test
     [TestFixture]
     public class CorsFixture : IntegrationTest
     {
-        private RestRequest BuildRequest()
+        private RestRequest BuildGet(string route = "artist")
         {
-            var request = new RestRequest("artist");
+            var request = new RestRequest(route, Method.GET);
             request.AddHeader(AccessControlHeaders.RequestMethod, "POST");
 
             return request;
         }
 
-        [Test]
+        private RestRequest BuildOptions(string route = "artist")
+        {
+            var request = new RestRequest(route, Method.OPTIONS);
+
+            return request;
+        }
+
+    [Test]
         public void should_not_have_allow_headers_in_response_when_not_included_in_the_request()
         {
-            var request = BuildRequest();
+            var request = BuildOptions();
-            var response = RestClient.Get(request);
+            var response = RestClient.Execute(request);
             
             response.Headers.Should().NotContain(h => h.Name == AccessControlHeaders.AllowHeaders);
         }
@@ -28,10 +35,10 @@ namespace NzbDrone.Integration.Test
         [Test]
         public void should_have_allow_headers_in_response_when_included_in_the_request()
         {
-            var request = BuildRequest();
+            var request = BuildOptions();
             request.AddHeader(AccessControlHeaders.RequestHeaders, "X-Test");
 
-            var response = RestClient.Get(request);
+            var response = RestClient.Execute(request);
 
             response.Headers.Should().Contain(h => h.Name == AccessControlHeaders.AllowHeaders);
         }
@@ -39,8 +46,8 @@ namespace NzbDrone.Integration.Test
         [Test]
         public void should_have_allow_origin_in_response()
         {
-            var request = BuildRequest();
+            var request = BuildOptions();
-            var response = RestClient.Get(request);
+            var response = RestClient.Execute(request);
 
             response.Headers.Should().Contain(h => h.Name == AccessControlHeaders.AllowOrigin);
         }
@@ -48,10 +55,37 @@ namespace NzbDrone.Integration.Test
         [Test]
         public void should_have_allow_methods_in_response()
         {
-            var request = BuildRequest();
+            var request = BuildOptions();
-            var response = RestClient.Get(request);
+            var response = RestClient.Execute(request);
 
             response.Headers.Should().Contain(h => h.Name == AccessControlHeaders.AllowMethods);
         }
+
+        [Test]
+        public void should_not_have_allow_methods_in_non_options_request()
+        {
+            var request = BuildGet();
+            var response = RestClient.Execute(request);
+
+            response.Headers.Should().NotContain(h => h.Name == AccessControlHeaders.AllowMethods);
+        }
+
+        [Test]
+        public void should_have_allow_origin_in_non_options_request()
+        {
+            var request = BuildGet();
+            var response = RestClient.Execute(request);
+
+            response.Headers.Should().Contain(h => h.Name == AccessControlHeaders.AllowOrigin);
+        }
+
+        [Test]
+        public void should_not_have_allow_origin_in_non_api_request()
+        {
+            var request = BuildGet("../abc");
+            var response = RestClient.Execute(request);
+
+            response.Headers.Should().NotContain(h => h.Name == AccessControlHeaders.AllowOrigin);
+        }
     }
 }