From 18c3345e47663a358a042652e697b988d6a380eb Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 25 Nov 2015 19:39:24 +0100 Subject: [PATCH] Fixes U4-7459 XSRF protection bypass - ensures tokens are checked for the non-editor api controllers --- .../umbraco_client/Application/Extensions.js | 16 +++++ ...dateMvcAngularAntiForgeryTokenAttribute.cs | 61 +++++++++++++++++++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + .../Filters/AngularAntiForgeryHelper.cs | 45 +++++++++----- .../WebServices/BulkPublishController.cs | 1 + .../WebServices/DomainsApiController.cs | 2 + .../ExamineManagementApiController.cs | 2 + .../WebServices/SaveFileController.cs | 1 + .../WebServices/XmlDataIntegrityController.cs | 3 +- 9 files changed, 116 insertions(+), 16 deletions(-) create mode 100644 src/Umbraco.Web/Mvc/ValidateMvcAngularAntiForgeryTokenAttribute.cs diff --git a/src/Umbraco.Web.UI/umbraco_client/Application/Extensions.js b/src/Umbraco.Web.UI/umbraco_client/Application/Extensions.js index 66c97c4db7..63870fad08 100644 --- a/src/Umbraco.Web.UI/umbraco_client/Application/Extensions.js +++ b/src/Umbraco.Web.UI/umbraco_client/Application/Extensions.js @@ -357,4 +357,20 @@ }); + //This sets the default jquery ajax headers to include our csrf token, we + // need to user the beforeSend method because our token changes per user/login so + // it cannot be static + $.ajaxSetup({ + beforeSend: function (xhr) { + + function getCookie(name) { + var value = "; " + document.cookie; + var parts = value.split("; " + name + "="); + if (parts.length === 2) return parts.pop().split(";").shift(); + } + + xhr.setRequestHeader("X-XSRF-TOKEN", getCookie("XSRF-TOKEN")); + } + }); + })(jQuery); \ No newline at end of file diff --git a/src/Umbraco.Web/Mvc/ValidateMvcAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web/Mvc/ValidateMvcAngularAntiForgeryTokenAttribute.cs new file mode 100644 index 0000000000..977f37473b --- /dev/null +++ b/src/Umbraco.Web/Mvc/ValidateMvcAngularAntiForgeryTokenAttribute.cs @@ -0,0 +1,61 @@ +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Security.Claims; +using System.Web.Mvc; +using Umbraco.Web.WebApi.Filters; + +namespace Umbraco.Web.Mvc +{ + /// + /// A filter to check for the csrf token based on Angular's standard approach + /// + /// + /// Code derived from http://ericpanorel.net/2013/07/28/spa-authentication-and-csrf-mvc4-antiforgery-implementation/ + /// + /// If the authentication type is cookie based, then this filter will execute, otherwise it will be disabled + /// + public sealed class ValidateMvcAngularAntiForgeryTokenAttribute : ActionFilterAttribute + { + public override void OnActionExecuting(ActionExecutingContext filterContext) + { + var userIdentity = filterContext.HttpContext.User.Identity as ClaimsIdentity; + if (userIdentity != null) + { + //if there is not CookiePath claim, then exist + if (userIdentity.HasClaim(x => x.Type == ClaimTypes.CookiePath) == false) + { + base.OnActionExecuting(filterContext); + return; + } + } + + string failedReason; + var headers = new List>>(); + foreach (var key in filterContext.HttpContext.Request.Headers.AllKeys) + { + if (headers.Any(x => x.Key == key)) + { + var found = headers.First(x => x.Key == key); + found.Value.Add(filterContext.HttpContext.Request.Headers[key]); + } + else + { + headers.Add(new KeyValuePair>(key, new List { filterContext.HttpContext.Request.Headers[key] })); + } + } + var cookie = filterContext.HttpContext.Request.Cookies[AngularAntiForgeryHelper.CsrfValidationCookieName]; + if (AngularAntiForgeryHelper.ValidateHeaders( + headers.Select(x => new KeyValuePair>(x.Key, x.Value)).ToArray(), + cookie == null ? "" : cookie.Value, + out failedReason) == false) + { + var result = new HttpStatusCodeResult(HttpStatusCode.ExpectationFailed); + filterContext.Result = result; + return; + } + + base.OnActionExecuting(filterContext); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 1edf654af6..f84a2872ac 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -306,6 +306,7 @@ + diff --git a/src/Umbraco.Web/WebApi/Filters/AngularAntiForgeryHelper.cs b/src/Umbraco.Web/WebApi/Filters/AngularAntiForgeryHelper.cs index d48cd66077..962183f7ef 100644 --- a/src/Umbraco.Web/WebApi/Filters/AngularAntiForgeryHelper.cs +++ b/src/Umbraco.Web/WebApi/Filters/AngularAntiForgeryHelper.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; using System.Net.Http; using System.Net.Http.Headers; @@ -28,6 +30,8 @@ namespace Umbraco.Web.WebApi.Filters /// public const string AngularHeadername = "X-XSRF-TOKEN"; + + /// /// Returns 2 tokens - one for the cookie value and one that angular should set as the header value /// @@ -64,13 +68,10 @@ namespace Umbraco.Web.WebApi.Filters return true; } - /// - /// Validates the headers/cookies passed in for the request - /// - /// - /// - /// - public static bool ValidateHeaders(HttpRequestHeaders requestHeaders, out string failedReason) + internal static bool ValidateHeaders( + KeyValuePair>[] requestHeaders, + string cookieToken, + out string failedReason) { failedReason = ""; @@ -85,12 +86,7 @@ namespace Umbraco.Web.WebApi.Filters .Select(z => z.Value) .SelectMany(z => z) .FirstOrDefault(); - - var cookieToken = requestHeaders - .GetCookies() - .Select(c => c[CsrfValidationCookieName]) - .FirstOrDefault(); - + // both header and cookie must be there if (cookieToken == null || headerToken == null) { @@ -98,13 +94,32 @@ namespace Umbraco.Web.WebApi.Filters return false; } - if (ValidateTokens(cookieToken.Value, headerToken) == false) + if (ValidateTokens(cookieToken, headerToken) == false) { failedReason = "Invalid token"; return false; } - + return true; } + + /// + /// Validates the headers/cookies passed in for the request + /// + /// + /// + /// + public static bool ValidateHeaders(HttpRequestHeaders requestHeaders, out string failedReason) + { + var cookieToken = requestHeaders + .GetCookies() + .Select(c => c[CsrfValidationCookieName]) + .FirstOrDefault(); + + return ValidateHeaders( + requestHeaders.ToDictionary(x => x.Key, x => x.Value).ToArray(), + cookieToken == null ? null : cookieToken.Value, + out failedReason); + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/WebServices/BulkPublishController.cs b/src/Umbraco.Web/WebServices/BulkPublishController.cs index 30a609f6ff..cc9a153e07 100644 --- a/src/Umbraco.Web/WebServices/BulkPublishController.cs +++ b/src/Umbraco.Web/WebServices/BulkPublishController.cs @@ -15,6 +15,7 @@ namespace Umbraco.Web.WebServices /// /// A REST controller used for the publish dialog in order to publish bulk items at once /// + [ValidateMvcAngularAntiForgeryToken] public class BulkPublishController : UmbracoAuthorizedController { /// diff --git a/src/Umbraco.Web/WebServices/DomainsApiController.cs b/src/Umbraco.Web/WebServices/DomainsApiController.cs index 4f3509845d..8c146f2670 100644 --- a/src/Umbraco.Web/WebServices/DomainsApiController.cs +++ b/src/Umbraco.Web/WebServices/DomainsApiController.cs @@ -13,6 +13,7 @@ using Umbraco.Web.WebApi; //using umbraco.cms.businesslogic.language; using umbraco.BusinessLogic.Actions; using umbraco.cms.businesslogic.web; +using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.WebServices { @@ -20,6 +21,7 @@ namespace Umbraco.Web.WebServices /// A REST controller used for managing domains. /// /// Nothing to do with Active Directory. + [ValidateAngularAntiForgeryToken] public class DomainsApiController : UmbracoAuthorizedApiController { [HttpPost] diff --git a/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs b/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs index 01e9f30254..31ef4d23f2 100644 --- a/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs +++ b/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs @@ -13,9 +13,11 @@ using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Web.Search; using Umbraco.Web.WebApi; +using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.WebServices { + [ValidateAngularAntiForgeryToken] public class ExamineManagementApiController : UmbracoAuthorizedApiController { /// diff --git a/src/Umbraco.Web/WebServices/SaveFileController.cs b/src/Umbraco.Web/WebServices/SaveFileController.cs index f18f690b87..fb2c90a8c3 100644 --- a/src/Umbraco.Web/WebServices/SaveFileController.cs +++ b/src/Umbraco.Web/WebServices/SaveFileController.cs @@ -26,6 +26,7 @@ namespace Umbraco.Web.WebServices /// This isn't fully implemented yet but we should migrate all of the logic in the umbraco.presentation.webservices.codeEditorSave /// over to this controller. /// + [ValidateMvcAngularAntiForgeryToken] public class SaveFileController : UmbracoAuthorizedController { /// diff --git a/src/Umbraco.Web/WebServices/XmlDataIntegrityController.cs b/src/Umbraco.Web/WebServices/XmlDataIntegrityController.cs index 6c1859dbfa..66951d12f2 100644 --- a/src/Umbraco.Web/WebServices/XmlDataIntegrityController.cs +++ b/src/Umbraco.Web/WebServices/XmlDataIntegrityController.cs @@ -4,10 +4,11 @@ using Umbraco.Core; using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence; using Umbraco.Web.WebApi; +using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.WebServices { - + [ValidateAngularAntiForgeryToken] public class XmlDataIntegrityController : UmbracoAuthorizedApiController { [HttpPost]