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

>
> DWORD len = GetLengthSid(pTokenUser->User.Sid)
> *ppSidUser = (PSID) HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len);
> CopySid(len, *ppSidUser, pTokenUser->User.Sid);
>

I attached a patch for this.  Although, I did not use CopySid.  Instead,
I changed GetUserSid to GetTokenUser.  AddUserToDacl() is the only
function making use of GetUserSid(), so this change won't break
anything.  The benefit to this approach over my first suggestion is that
it avoids an unneeded HeapAlloc(sid), CopySid(sid) ... and its cleaner.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/port/exec.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/port/exec.c,v
retrieving revision 1.63
diff -C6 -r1.63 exec.c
*** src/port/exec.c    11 Jun 2009 14:49:15 -0000    1.63
--- src/port/exec.c    23 Jun 2009 14:57:46 -0000
***************
*** 53,65 ****

  static int    validate_exec(const char *path);
  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

  /*
   * validate_exec -- validate "path" as an executable file
   *
   * returns 0 if the file is found and no error is encountered.
--- 53,65 ----

  static int    validate_exec(const char *path);
  static int    resolve_symlinks(char *path);
  static char *pipe_read_line(char *cmd, char *line, int maxsize);

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

  /*
   * validate_exec -- validate "path" as an executable file
   *
   * returns 0 if the file is found and no error is encountered.
***************
*** 694,706 ****
      ACCESS_ALLOWED_ACE *pace;
      DWORD        dwNewAclSize;
      DWORD        dwSize = 0;
      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;
      BOOL        ret = FALSE;

      /* Get the token for the process */
--- 694,706 ----
      ACCESS_ALLOWED_ACE *pace;
      DWORD        dwNewAclSize;
      DWORD        dwSize = 0;
      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;
      BOOL        ret = FALSE;

      /* Get the token for the process */
***************
*** 741,761 ****
                             AclSizeInformation))
      {
          log_error("could not get ACL information: %lu", GetLastError());
          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);
      if (pacl == NULL)
      {
          log_error("could not allocate %lu bytes of memory", dwNewAclSize);
--- 741,764 ----
                             AclSizeInformation))
      {
          log_error("could not get ACL information: %lu", GetLastError());
          goto cleanup;
      }

!     /* Get the user token for the current user. This provides us with the
!      * user's SID which 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);
      if (pacl == NULL)
      {
          log_error("could not allocate %lu bytes of memory", dwNewAclSize);
***************
*** 782,794 ****
              log_error("could not add ACE: %lu", GetLastError());
              goto cleanup;
          }
      }

      /* 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;
      }

      /* Set the new DACL in the token */
--- 785,797 ----
              log_error("could not add ACE: %lu", GetLastError());
              goto cleanup;
          }
      }

      /* 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;
      }

      /* Set the new DACL in the token */
***************
*** 800,813 ****
          goto cleanup;
      }

      ret = TRUE;

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

      if (pacl)
          LocalFree((HLOCAL) pacl);

      if (ptdd)
          LocalFree((HLOCAL) ptdd);
--- 803,816 ----
          goto cleanup;
      }

      ret = TRUE;

  cleanup:
!     if (pTokenUser)
!         HeapFree(GetProcessHeap(), 0, pTokenUser);

      if (pacl)
          LocalFree((HLOCAL) pacl);

      if (ptdd)
          LocalFree((HLOCAL) ptdd);
***************
*** 816,873 ****
          CloseHandle(hToken);

      return ret;
  }

  /*
!  * 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;
              }
          }
          else
          {
              log_error("could not get token information buffer size: %lu", GetLastError());
              return FALSE;
          }
      }

!     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;
  }

  #endif
--- 819,871 ----
          CloseHandle(hToken);

      return ret;
  }

  /*
!  * GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser)
   *
!  * Get the user's token information from a process token.  If the
!  * function succeeds, it returns TRUE and *ppTokenUser is assigned
!  * to memory allocated with HeapAlloc() ... free with HeapFree().
!  * If the function fails, it returns FALSE and *ppTokenUser will
!  * point to NULL.
   */
  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) HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dwLength);

!             if (*ppTokenUser == NULL)
              {
                  log_error("could not allocate %lu bytes of memory", dwLength);
                  return FALSE;
              }
          }
          else
          {
              log_error("could not get token information buffer size: %lu", GetLastError());
              return FALSE;
          }
      }

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

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

      return TRUE;
  }

  #endif