Re: GetTokenInformation() and FreeSid() at port/exec.c

Andrew Chernow wrote:
>
>>
>> How about something like this? I switched to using LocalAlloc() in all
>> places to be consistent, instead of mixing heap and local. (Though per
>> doc, LocalAlloc is actually a wrapper for HeapAlloc in win32).
>
> Our patches crossed.  Although, in my patch I left the allocation scheme
> alone since I wasn't sure if someone wanted that way.  I'd suggest
> malloc and free if your going to change it.  The only time I use an MS
> allocater is when a win32 api function specifically states it must be used.

Attached is a mix of our two patches. How does that look to you?


--
 Magnus Hagander
 Self: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
*** a/src/port/exec.c
--- b/src/port/exec.c
***************
*** 56,62 **** static int    resolve_symlinks(char *path);
  static char *pipe_read_line(char *cmd, char *line, int maxsize);

  #ifdef WIN32
! static BOOL GetUserSid(PSID *ppSidUser, HANDLE hToken);
  #endif

  /*
--- 56,62 ----
  static char *pipe_read_line(char *cmd, char *line, int maxsize);

  #ifdef WIN32
! static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser);
  #endif

  /*
***************
*** 697,703 **** AddUserToDacl(HANDLE hProcess)
      DWORD        dwTokenInfoLength = 0;
      HANDLE        hToken = NULL;
      PACL        pacl = NULL;
!     PSID        psidUser = NULL;
      TOKEN_DEFAULT_DACL tddNew;
      TOKEN_DEFAULT_DACL *ptdd = NULL;
      TOKEN_INFORMATION_CLASS tic = TokenDefaultDacl;
--- 697,703 ----
      DWORD        dwTokenInfoLength = 0;
      HANDLE        hToken = NULL;
      PACL        pacl = NULL;
!     PTOKEN_USER    pTokenUser = NULL;
      TOKEN_DEFAULT_DACL tddNew;
      TOKEN_DEFAULT_DACL *ptdd = NULL;
      TOKEN_INFORMATION_CLASS tic = TokenDefaultDacl;
***************
*** 744,758 **** AddUserToDacl(HANDLE hProcess)
          goto cleanup;
      }

!     /* Get the SID for the current user. We need to add this to the ACL. */
!     if (!GetUserSid(&psidUser, hToken))
      {
!         log_error("could not get user SID: %lu", GetLastError());
          goto cleanup;
      }

      /* Figure out the size of the new ACL */
!     dwNewAclSize = asi.AclBytesInUse + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(psidUser) -sizeof(DWORD);

      /* Allocate the ACL buffer & initialize it */
      pacl = (PACL) LocalAlloc(LPTR, dwNewAclSize);
--- 744,762 ----
          goto cleanup;
      }

!     /*
!      * Get the user token for the current user, which provides us with the
!      * SID that is needed for creating the ACL.
!      */
!     if (!GetTokenUser(hToken, &pTokenUser))
      {
!         log_error("could not get user token: %lu", GetLastError());
          goto cleanup;
      }

      /* Figure out the size of the new ACL */
!     dwNewAclSize = asi.AclBytesInUse + sizeof(ACCESS_ALLOWED_ACE) +
!         GetLengthSid(pTokenUser->User.Sid) -sizeof(DWORD);

      /* Allocate the ACL buffer & initialize it */
      pacl = (PACL) LocalAlloc(LPTR, dwNewAclSize);
***************
*** 785,791 **** AddUserToDacl(HANDLE hProcess)
      }

      /* Add the new ACE for the current user */
!     if (!AddAccessAllowedAce(pacl, ACL_REVISION, GENERIC_ALL, psidUser))
      {
          log_error("could not add access allowed ACE: %lu", GetLastError());
          goto cleanup;
--- 789,795 ----
      }

      /* Add the new ACE for the current user */
!     if (!AddAccessAllowedAce(pacl, ACL_REVISION, GENERIC_ALL, pTokenUser->User.Sid))
      {
          log_error("could not add access allowed ACE: %lu", GetLastError());
          goto cleanup;
***************
*** 803,810 **** AddUserToDacl(HANDLE hProcess)
      ret = TRUE;

  cleanup:
!     if (psidUser)
!         FreeSid(psidUser);

      if (pacl)
          LocalFree((HLOCAL) pacl);
--- 807,814 ----
      ret = TRUE;

  cleanup:
!     if (pTokenUser)
!         LocalFree((HLOCAL) pTokenUser);

      if (pacl)
          LocalFree((HLOCAL) pacl);
***************
*** 819,846 **** cleanup:
  }

  /*
!  * GetUserSid*PSID *ppSidUser, HANDLE hToken)
   *
!  * Get the SID for the current user
   */
  static BOOL
! GetUserSid(PSID *ppSidUser, HANDLE hToken)
  {
      DWORD        dwLength;
-     PTOKEN_USER pTokenUser = NULL;


      if (!GetTokenInformation(hToken,
                               TokenUser,
!                              pTokenUser,
                               0,
                               &dwLength))
      {
          if (GetLastError() == ERROR_INSUFFICIENT_BUFFER)
          {
!             pTokenUser = (PTOKEN_USER) HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dwLength);

!             if (pTokenUser == NULL)
              {
                  log_error("could not allocate %lu bytes of memory", dwLength);
                  return FALSE;
--- 823,853 ----
  }

  /*
!  * GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser)
!  *
!  * Get the users token information from a process token.
   *
!  * The caller of this function is responsible for calling LocalFree() on the
!  * returned TOKEN_USER memory.
   */
  static BOOL
! GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser)
  {
      DWORD        dwLength;

+     *ppTokenUser = NULL;

      if (!GetTokenInformation(hToken,
                               TokenUser,
!                              NULL,
                               0,
                               &dwLength))
      {
          if (GetLastError() == ERROR_INSUFFICIENT_BUFFER)
          {
!             *ppTokenUser = (PTOKEN_USER) LocalAlloc(LPTR, dwLength);

!             if (*ppTokenUser == NULL)
              {
                  log_error("could not allocate %lu bytes of memory", dwLength);
                  return FALSE;
***************
*** 855,872 **** GetUserSid(PSID *ppSidUser, HANDLE hToken)

      if (!GetTokenInformation(hToken,
                               TokenUser,
!                              pTokenUser,
                               dwLength,
                               &dwLength))
      {
!         HeapFree(GetProcessHeap(), 0, pTokenUser);
!         pTokenUser = NULL;

          log_error("could not get token information: %lu", GetLastError());
          return FALSE;
      }

!     *ppSidUser = pTokenUser->User.Sid;
      return TRUE;
  }

--- 862,879 ----

      if (!GetTokenInformation(hToken,
                               TokenUser,
!                              *ppTokenUser,
                               dwLength,
                               &dwLength))
      {
!         LocalFree(*ppTokenUser);
!         *ppTokenUser = NULL;

          log_error("could not get token information: %lu", GetLastError());
          return FALSE;
      }

!     /* Memory in *ppTokenUser is LocalFree():d by the caller */
      return TRUE;
  }