From 8b24e1b0483ddb69321fe0074561849327e344f3 Mon Sep 17 00:00:00 2001
From: Eric Kohl <eric.kohl@reactos.org>
Date: Mon, 4 Jul 2016 21:37:24 +0000
Subject: [PATCH] [SERVICES] Fix yet another MSDN bug: ChangeServiceConfig2
 ALWAYS requires SERVICE_CHANGE_CONFIG and SERVICE_START access rights and an
 enabled SE_SHUTDOWN_PRIVILEGE privilege when you try to set the failure
 actions. Otherwise, you will just get a STATUS_ACCESS_DENIED.

- Remove the access and privilege checks from ScmSetFailureActions and move them to RChangeServiceConfig2W.

svn path=/trunk/; revision=71814
---
 reactos/base/system/services/rpcserver.c | 78 ++++++++----------------
 1 file changed, 24 insertions(+), 54 deletions(-)

diff --git a/reactos/base/system/services/rpcserver.c b/reactos/base/system/services/rpcserver.c
index db529e9132b..7a773bec0ea 100644
--- a/reactos/base/system/services/rpcserver.c
+++ b/reactos/base/system/services/rpcserver.c
@@ -5018,18 +5018,13 @@ DWORD RChangeServiceConfig2A(
 
 
 static DWORD
-ScmSetFailureActions(PSERVICE_HANDLE hSvc,
-                     PSERVICE lpService,
-                     HKEY hServiceKey,
+ScmSetFailureActions(HKEY hServiceKey,
                      LPSERVICE_FAILURE_ACTIONSW lpFailureActions)
 {
     LPSERVICE_FAILURE_ACTIONSW lpReadBuffer = NULL;
     LPSERVICE_FAILURE_ACTIONSW lpWriteBuffer = NULL;
-    BOOL bIsActionRebootSet = FALSE;
-    DWORD dwDesiredAccess = SERVICE_CHANGE_CONFIG;
     DWORD dwRequiredSize = 0;
     DWORD dwType = 0;
-    DWORD i = 0;
     DWORD dwError;
 
     /* There is nothing to be done if we have no failure actions */
@@ -5037,48 +5032,7 @@ ScmSetFailureActions(PSERVICE_HANDLE hSvc,
         return ERROR_SUCCESS;
 
     /*
-     * 1- Check whether or not we can set
-     *    failure actions for this service.
-     */
-
-    /* Failure actions can only be set for Win32 services, not for drivers */
-    if (lpService->Status.dwServiceType & SERVICE_DRIVER)
-        return ERROR_CANNOT_DETECT_DRIVER_FAILURE;
-
-    /*
-     * If the service controller handles the SC_ACTION_RESTART action,
-     * hService must have the SERVICE_START access right.
-     *
-     * If you specify SC_ACTION_REBOOT, the caller must have the
-     * SE_SHUTDOWN_NAME privilege.
-     */
-    if (lpFailureActions->cActions > 0 &&
-        lpFailureActions->lpsaActions != NULL)
-    {
-        for (i = 0; i < lpFailureActions->cActions; ++i)
-        {
-           if (lpFailureActions->lpsaActions[i].Type == SC_ACTION_RESTART)
-               dwDesiredAccess |= SERVICE_START;
-           else if (lpFailureActions->lpsaActions[i].Type == SC_ACTION_REBOOT)
-               bIsActionRebootSet = TRUE;
-        }
-    }
-
-    /* Re-check the access rights */
-    if (!RtlAreAllAccessesGranted(hSvc->Handle.DesiredAccess,
-                                  dwDesiredAccess))
-    {
-        DPRINT1("Insufficient access rights! 0x%lx\n", hSvc->Handle.DesiredAccess);
-        return ERROR_ACCESS_DENIED;
-    }
-
-    /* FIXME: Check if the caller has the SE_SHUTDOWN_NAME privilege */
-    if (bIsActionRebootSet)
-    {
-    }
-
-    /*
-     * 2- Retrieve the original value of FailureActions.
+     * 1- Retrieve the original value of FailureActions.
      */
 
     /* Query value length */
@@ -5145,7 +5099,7 @@ ScmSetFailureActions(PSERVICE_HANDLE hSvc,
     lpReadBuffer->lpCommand = NULL;
 
     /*
-     * 3- Initialize the new value to set.
+     * 2- Initialize the new value to set.
      */
 
     dwRequiredSize = sizeof(SERVICE_FAILURE_ACTIONSW);
@@ -5292,6 +5246,7 @@ DWORD RChangeServiceConfig2W(
     PSERVICE_HANDLE hSvc;
     PSERVICE lpService = NULL;
     HKEY hServiceKey = NULL;
+    ACCESS_MASK RequiredAccess = SERVICE_CHANGE_CONFIG;
 
     DPRINT("RChangeServiceConfig2W() called\n");
     DPRINT("dwInfoLevel = %lu\n", Info.dwInfoLevel);
@@ -5302,17 +5257,27 @@ DWORD RChangeServiceConfig2W(
     hSvc = ScmGetServiceFromHandle(hService);
     if (hSvc == NULL)
     {
-        DPRINT1("Invalid service handle!\n");
+        DPRINT("Invalid service handle!\n");
         return ERROR_INVALID_HANDLE;
     }
 
+    if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
+        RequiredAccess |= SERVICE_START;
+
+    /* Check the access rights */
     if (!RtlAreAllAccessesGranted(hSvc->Handle.DesiredAccess,
-                                  SERVICE_CHANGE_CONFIG))
+                                  RequiredAccess))
     {
         DPRINT("Insufficient access rights! 0x%lx\n", hSvc->Handle.DesiredAccess);
         return ERROR_ACCESS_DENIED;
     }
 
+    if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
+    {
+        /* FIXME: Check if the caller has the SE_SHUTDOWN_NAME privilege */
+
+    }
+
     lpService = hSvc->ServiceEntry;
     if (lpService == NULL)
     {
@@ -5320,6 +5285,13 @@ DWORD RChangeServiceConfig2W(
         return ERROR_INVALID_HANDLE;
     }
 
+    /* Failure actions can only be set for Win32 services, not for drivers */
+    if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
+    {
+        if (lpService->Status.dwServiceType & SERVICE_DRIVER)
+            return ERROR_CANNOT_DETECT_DRIVER_FAILURE;
+    }
+
     /* Lock the service database exclusively */
     ScmLockDatabaseExclusive();
 
@@ -5372,9 +5344,7 @@ DWORD RChangeServiceConfig2W(
     }
     else if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
     {
-        dwError = ScmSetFailureActions(hSvc,
-                                       lpService,
-                                       hServiceKey,
+        dwError = ScmSetFailureActions(hServiceKey,
                                        (LPSERVICE_FAILURE_ACTIONSW)Info.psfa);
     }