From af29f614786a4eec91b1af7c7b18fa77e11d4360 Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Fri, 29 Nov 2013 11:10:25 +1100
Subject: [PATCH] Fixes csrf issue

---
 .../config/ClientDependency.config            |  2 +-
 src/Umbraco.Web.UI/install/Default.aspx.cs    |  4 +++-
 src/Umbraco.Web.UI/umbraco/logout.aspx        | 17 +++-----------
 src/Umbraco.Web.UI/umbraco/umbraco.aspx       | 13 +++++++----
 .../Application/UmbracoApplicationActions.js  | 21 +++++++++++------
 .../UI/Pages/UmbracoEnsuredPage.cs            |  4 ++--
 .../install/LegacyClasses.cs                  |  2 +-
 .../umbraco/logout.aspx.cs                    | 23 ++++++++++++++-----
 .../BasePages/BasePage.cs                     | 12 ++++++++++
 .../BasePages/UmbracoEnsuredPage.cs           |  4 ++--
 src/umbraco.cms/Actions/ActionQuit.cs         |  1 +
 11 files changed, 65 insertions(+), 38 deletions(-)
diff --git a/src/Umbraco.Web.UI/config/ClientDependency.config b/src/Umbraco.Web.UI/config/ClientDependency.config
index a95c67b06d..3b80c0d7dd 100644
--- a/src/Umbraco.Web.UI/config/ClientDependency.config
+++ b/src/Umbraco.Web.UI/config/ClientDependency.config
@@ -10,7 +10,7 @@ NOTES:
 * Compression/Combination/Minification is not enabled unless debug="false" is specified on the 'compiliation' element in the web.config
 * A new version will invalidate both client and server cache and create new persisted files
 -->
-<clientDependency version="8" fileDependencyExtensions=".js,.css" loggerType="Umbraco.Web.UI.CdfLogger, umbraco">
+<clientDependency version="9" fileDependencyExtensions=".js,.css" loggerType="Umbraco.Web.UI.CdfLogger, umbraco">
 
   <!-- 
     This section is used for Web Forms only, the enableCompositeFiles="true" is optional and by default is set to true.
diff --git a/src/Umbraco.Web.UI/install/Default.aspx.cs b/src/Umbraco.Web.UI/install/Default.aspx.cs
index a53429c2b6..345a0bc841 100644
--- a/src/Umbraco.Web.UI/install/Default.aspx.cs
+++ b/src/Umbraco.Web.UI/install/Default.aspx.cs
@@ -67,7 +67,9 @@ namespace Umbraco.Web.UI.Install
                 
                 if (result == ValidateRequestAttempt.FailedTimedOut || result == ValidateRequestAttempt.FailedNoPrivileges)
                 {
-                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl));
+                    Response.Redirect(
+                        //We must add the token to prevent CSRF attacks since the logout occurs on a GET not a POST
+                        SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl) + "&t=" + Security.UmbracoUserContextId);
                 }
             }
 
diff --git a/src/Umbraco.Web.UI/umbraco/logout.aspx b/src/Umbraco.Web.UI/umbraco/logout.aspx
index 7828fb3d8e..cb01b76af6 100644
--- a/src/Umbraco.Web.UI/umbraco/logout.aspx
+++ b/src/Umbraco.Web.UI/umbraco/logout.aspx
@@ -1,21 +1,10 @@
-<%@ Page language="c#" Codebehind="logout.aspx.cs" AutoEventWireup="True" Inherits="umbraco.logout" %>
+<%@ Page language="c#" AutoEventWireup="True" Inherits="umbraco.logout" %>
 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" > 
 
 <html>
   <head>
-    <title>logout</title>
-    <meta name="GENERATOR" Content="Microsoft Visual Studio .NET 7.1">
-    <meta name="CODE_LANGUAGE" Content="C#">
-    <meta name=vs_defaultClientScript content="JavaScript">
-    <meta name=vs_targetSchema content="http://schemas.microsoft.com/intellisense/ie5">
+    <title>Logout</title>
   </head>
-  <body MS_POSITIONING="GridLayout">
-	
-    <form id="Form1" method="post" runat="server">
-		<script>
-			window.top.location.href='login.aspx?redir=<%=Server.UrlEncode(Request["redir"]) %>';
-		</script>
-     </form>
-	
+  <body>	
   </body>
 </html>
diff --git a/src/Umbraco.Web.UI/umbraco/umbraco.aspx b/src/Umbraco.Web.UI/umbraco/umbraco.aspx
index 0bcb810d20..409896d3e5 100644
--- a/src/Umbraco.Web.UI/umbraco/umbraco.aspx
+++ b/src/Umbraco.Web.UI/umbraco/umbraco.aspx
@@ -91,14 +91,19 @@
                 </div>
             </asp:Panel>
             <div class="topBarButtons">
+                
                 <button onclick="UmbClientMgr.appActions().launchAbout();" class="topBarButton">
                     <img src="images/aboutNew.png" alt="about" /><span><%=umbraco.ui.Text("general", "about")%></span></button>
                 <button onclick="UmbClientMgr.appActions().launchHelp('<%=UmbracoUser.Language%>', '<%=UmbracoUser.UserType.Name%>');"
                     class="topBarButton">
                     <img src="images/help.png" alt="Help" /><span><%=umbraco.ui.Text("general", "help")%></span></button>
-                <button onclick="UmbClientMgr.appActions().logout();" class="topBarButton">
-                    <img src="images/logout.png" alt="Log out" /><span><%=umbraco.ui.Text("general", "logout")%>:
-                        <%=UmbracoUser.Name%></span></button>
+                <form action="logout.aspx" method="get" style="display: inline;" >
+                    <button class="topBarButton" type="submit">
+                        <img src="images/logout.png" alt="Log out" />
+                        <span><%=umbraco.ui.Text("general", "logout")%>:<%=UmbracoUser.Name%></span>
+                    </button>
+                    <input type="hidden" value="<%=Security.UmbracoUserContextId %>" name="t" id="t"/>
+                </form>
             </div>
         </div>
     </div>
@@ -341,7 +346,7 @@
         function umbracoSessionLogout() {
 
             //alert('Session has expired on server - can\'t renew. Logging out!');
-            top.document.location.href = 'logout.aspx';
+            top.document.location.href = 'logout.aspx?t=<%=Security.UmbracoUserContextId%>';
         }
 
         function blink($target) {
diff --git a/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoApplicationActions.js b/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoApplicationActions.js
index 0e9b508784..4920173642 100644
--- a/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoApplicationActions.js
+++ b/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoApplicationActions.js
@@ -10,7 +10,7 @@ Umbraco.Application.Actions = function () {
     /// <summary>
     /// Application actions actions for the context menu, help dialogs, logout, etc...
     /// This class supports an event listener model. Currently the available events are:
-    /// "nodeDeleting","nodeDeleted","nodeRefresh","beforeLogout"
+    /// "nodeDeleting","nodeDeleted","nodeRefresh"
     /// </summary>
 
     return {
@@ -75,13 +75,17 @@ Umbraco.Application.Actions = function () {
                 alert('Not supported - please create by right clicking the parentnode and choose new...');
         },
 
-        logout: function () {
-            /// <summary>Logs the user out</summary>
+        logout: function (t) {
+            
+            if (!t) {
+                throw "The security token must be set in order to log a user out using this method";
+            }
+
             if (confirm(UmbClientMgr.uiKeys()["defaultdialogs_confirmlogout"])) {
                 //raise beforeLogout event
                 jQuery(window.top).trigger("beforeLogout", []);
 
-                document.location.href = 'logout.aspx';
+                document.location.href = 'logout.aspx?t=' + t;
             }
             return false;
         },
@@ -240,11 +244,14 @@ Umbraco.Application.Actions = function () {
             }
         },
 
-        actionQuit: function () {
-            /// <summary></summary>
+        actionQuit: function (t) {
+            
+            if (!t) {
+                throw "The security token must be set in order to log a user out using this method";
+            }
 
             if (confirm(uiKeys['defaultdialogs_confirmlogout'] + '\n\n'))
-                document.location.href = 'logout.aspx';
+                document.location.href = 'logout.aspx?t=' + t;
         },
 
         actionRePublish: function () {
diff --git a/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs b/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs
index be55343696..5f138bad1e 100644
--- a/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs
+++ b/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs
@@ -46,9 +46,9 @@ namespace Umbraco.Web.UI.Pages
 
                 // Some umbraco pages should not be loaded on timeout, but instead reload the main application in the top window. Like the treeview for instance
                 if (RedirectToUmbraco)
-                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?", true);
+                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?t=" + Security.UmbracoUserContextId, true);
                 else
-                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl), true);
+                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl) + "&t=" + Security.UmbracoUserContextId, true);
             }
         }
 
diff --git a/src/Umbraco.Web/umbraco.presentation/install/LegacyClasses.cs b/src/Umbraco.Web/umbraco.presentation/install/LegacyClasses.cs
index 9e04a183ea..d901951eae 100644
--- a/src/Umbraco.Web/umbraco.presentation/install/LegacyClasses.cs
+++ b/src/Umbraco.Web/umbraco.presentation/install/LegacyClasses.cs
@@ -113,7 +113,7 @@ namespace umbraco.presentation.install
 				}
 				catch (Exception)
 				{
-					Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl));
+					Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl) + "&t=" + umbracoUserContextID);
 				}
 
 				//set the first step to upgrade.
diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/logout.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/logout.aspx.cs
index 15f8be030f..10ace87f60 100644
--- a/src/Umbraco.Web/umbraco.presentation/umbraco/logout.aspx.cs
+++ b/src/Umbraco.Web/umbraco.presentation/umbraco/logout.aspx.cs
@@ -8,21 +8,32 @@ using System.Web.SessionState;
 using System.Web.UI;
 using System.Web.UI.WebControls;
 using System.Web.UI.HtmlControls;
+using Umbraco.Web;
+using Umbraco.Core;
 
 namespace umbraco
 {
     /// <summary>
     /// Summary description for logout.
     /// </summary>
-    public partial class logout : BasePages.BasePage
+    public class logout : BasePages.BasePage
     {
-        protected void Page_Load(object sender, System.EventArgs e)
+        protected override void OnInit(EventArgs e)
         {
-            // Put user code to initialize the page here
-            if (umbracoUserContextID != "")
-                base.ClearLogin();
+            base.OnInit(e);
+
+            //We need to check the token in the URL to ensure it is correct otherwise malicious GET requests using CSRF attacks
+            // can easily just log the user out.
+            var token = Request["t"];
+            //only perform the logout if the token matches
+            if (token.IsNullOrWhiteSpace() == false && token == umbracoUserContextID)
+            {
+                ClearLogin();
+            }
+
+            //redirect home
+            Response.Redirect("Login.aspx?redir=" + Server.UrlEncode(Request["redir"]));
         }
 
-        protected System.Web.UI.HtmlControls.HtmlForm Form1;
     }
 }
diff --git a/src/umbraco.businesslogic/BasePages/BasePage.cs b/src/umbraco.businesslogic/BasePages/BasePage.cs
index c2aec995ce..1375c000fc 100644
--- a/src/umbraco.businesslogic/BasePages/BasePage.cs
+++ b/src/umbraco.businesslogic/BasePages/BasePage.cs
@@ -441,6 +441,18 @@ namespace umbraco.BasePages
             warning
         }
 
+        protected override void OnInit(EventArgs e)
+        {
+            base.OnInit(e);
+
+            //This must be set on each page to mitigate CSRF attacks which ensures that this unique token 
+            // is added to the viewstate of each request
+            if (umbracoUserContextID.IsNullOrWhiteSpace() == false)
+            {
+                ViewStateUserKey = umbracoUserContextID;
+            }
+        }
+
         /// <summary>
         /// Raises the <see cref="E:System.Web.UI.Control.Load"></see> event.
         /// </summary>
diff --git a/src/umbraco.businesslogic/BasePages/UmbracoEnsuredPage.cs b/src/umbraco.businesslogic/BasePages/UmbracoEnsuredPage.cs
index 4fa4a3fea3..e2ec0448b9 100644
--- a/src/umbraco.businesslogic/BasePages/UmbracoEnsuredPage.cs
+++ b/src/umbraco.businesslogic/BasePages/UmbracoEnsuredPage.cs
@@ -104,9 +104,9 @@ namespace umbraco.BasePages
 
                 // Some umbraco pages should not be loaded on timeout, but instead reload the main application in the top window. Like the treeview for instance
                 if (RedirectToUmbraco)
-                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?", true);
+                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?t=" + umbracoUserContextID, true);
                 else
-                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl), true);
+                    Response.Redirect(SystemDirectories.Umbraco + "/logout.aspx?redir=" + Server.UrlEncode(Request.RawUrl) + "&t=" + umbracoUserContextID, true);
             }
         }
     }
diff --git a/src/umbraco.cms/Actions/ActionQuit.cs b/src/umbraco.cms/Actions/ActionQuit.cs
index e9213ed7b5..93da6f4e8d 100644
--- a/src/umbraco.cms/Actions/ActionQuit.cs
+++ b/src/umbraco.cms/Actions/ActionQuit.cs
@@ -7,6 +7,7 @@ namespace umbraco.BusinessLogic.Actions
 	/// <summary>
 	/// This action is invoked when a user logs out
 	/// </summary>
+	[Obsolete("This should not be used and will be removed from the codebase in future versions")]
 	public class ActionQuit : IAction
 	{
 		//create singleton
