Fixed: Security Vulnerabilities allowing authentication to be bypassed (discovered by Kyle Neideck)
This commit is contained in:
commit
581320e3a4
|
@ -23,6 +23,8 @@ namespace NzbDrone.Api.Extensions.Pipelines
|
|||
|
||||
private void Handle(NancyContext context)
|
||||
{
|
||||
if (context.Request.Method == "OPTIONS") return;
|
||||
|
||||
if (_cacheableSpecification.IsCacheable(context))
|
||||
{
|
||||
context.Response.Headers.EnableCache();
|
||||
|
|
|
@ -2,6 +2,7 @@
|
|||
using System.Linq;
|
||||
using Nancy;
|
||||
using Nancy.Bootstrapper;
|
||||
using NzbDrone.Common.Extensions;
|
||||
|
||||
namespace NzbDrone.Api.Extensions.Pipelines
|
||||
{
|
||||
|
@ -11,10 +12,25 @@ namespace NzbDrone.Api.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,22 +42,40 @@ namespace NzbDrone.Api.Extensions.Pipelines
|
|||
|
||||
private static void ApplyResponseHeaders(Response response, Request request)
|
||||
{
|
||||
var allowedMethods = "GET, OPTIONS, PATCH, POST, PUT, DELETE";
|
||||
if (request.IsApiRequest())
|
||||
{
|
||||
// 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())
|
||||
{
|
||||
// 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"];
|
||||
}
|
||||
|
||||
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())
|
||||
{
|
||||
var requestedHeaders = request.Headers[AccessControlHeaders.RequestHeaders].Join(", ");
|
||||
|
||||
response.Headers.Add(AccessControlHeaders.AllowHeaders, requestedHeaders);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -33,7 +33,8 @@ namespace NzbDrone.Api.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)
|
||||
|
|
|
@ -36,5 +36,11 @@ namespace NzbDrone.Api.Extensions
|
|||
{
|
||||
return request.Path.StartsWith("/Content/", StringComparison.InvariantCultureIgnoreCase);
|
||||
}
|
||||
|
||||
public static bool IsSharedContentRequest(this Request request)
|
||||
{
|
||||
return request.Path.StartsWith("/MediaCover/", StringComparison.InvariantCultureIgnoreCase) ||
|
||||
request.Path.StartsWith("/Content/Images/", StringComparison.InvariantCultureIgnoreCase);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,3 +1,4 @@
|
|||
using System;
|
||||
using System.IO;
|
||||
using NLog;
|
||||
using NzbDrone.Common.Disk;
|
||||
|
@ -28,7 +29,9 @@ namespace NzbDrone.Api.Frontend.Mappers
|
|||
|
||||
public override bool CanHandle(string resourceUrl)
|
||||
{
|
||||
return resourceUrl.StartsWith("/Content") ||
|
||||
resourceUrl = resourceUrl.ToLowerInvariant();
|
||||
|
||||
return resourceUrl.StartsWith("/content") ||
|
||||
resourceUrl.EndsWith(".js") ||
|
||||
resourceUrl.EndsWith(".map") ||
|
||||
resourceUrl.EndsWith(".css") ||
|
||||
|
|
|
@ -8,19 +8,26 @@ namespace NzbDrone.Integration.Test
|
|||
[TestFixture]
|
||||
public class CorsFixture : IntegrationTest
|
||||
{
|
||||
private RestRequest BuildRequest()
|
||||
private RestRequest BuildGet(string route = "series")
|
||||
{
|
||||
var request = new RestRequest("series");
|
||||
var request = new RestRequest(route, Method.GET);
|
||||
request.AddHeader(AccessControlHeaders.RequestMethod, "POST");
|
||||
|
||||
return request;
|
||||
}
|
||||
|
||||
private RestRequest BuildOptions(string route = "series")
|
||||
{
|
||||
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 response = RestClient.Get(request);
|
||||
var request = BuildOptions();
|
||||
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 response = RestClient.Get(request);
|
||||
var request = BuildOptions();
|
||||
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 response = RestClient.Get(request);
|
||||
var request = BuildOptions();
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue