[Box Backup-dev] Reviewing code -- help!

[Charles Lecklider]

Chris Wilson wrote:
> Does this look OK to you?
> 
>         mSocketHandle = CreateFileW(...)
>     ...
>         DWORD Flags = PIPE_READMODE_MESSAGE | // put this end into
> message mode
>                 PIPE_WAIT;                    // put this end into
> blocking mode
> 
>         if (!SetNamedPipeHandleState(
>                 mSocketHandle,          // pipe handle
>                 &Flags,                 // mode flags
>                 NULL,                   // don't change the collection
> count
>                 NULL))                  // don't change the collect timeout
>         {
>                 ::syslog(LOG_ERR, "Failed to put pipe into message mode: "
>                         "error %d", GetLastError());
>                 THROW_EXCEPTION(ServerException, SocketOpenError)
>         }

Yes, that should do it.

>> Also, you need to think carefully about having a NULL SA on the pipe;
>> generally, this is a potential security problem.
> 
> 
> How about this?
> 
>         SECURITY_ATTRIBUTES Security;
>         Security.nLength = sizeof(SECURITY_ATTRIBUTES);
>         Security.lpSecurityDescriptor = NULL; // inherit from process
>         Security.bInheritHandle = FALSE; // don't pass to new processes
> 
>         mSocketHandle = CreateNamedPipeW(
>         ...
>                 &Security);                // use our security attributes

LOL!

You've replaced "NULL" by something functionally almost identical; the
only difference is that the handle doesn't get inherited.

You need to decide what users and groups need to be able to access the
pipe, and create a suitable security descriptor with a suitable DACL.
Along the way you'll encounter SIDs, ACEs, and ACLs - hours of fun and
joy await you!

-C