Being a software engineer nowadays isn’t what it used to be a couple of decades ago – a lot of questions have a solution or a code example available online. And while some of us may take code examples found on stackoverflow.com with a grain of salt, we’re less likely to do so for code provided by official sources, such as MSDN.
In this blog post I will talk about Microsoft’s example of how to implement a multithreaded pipe server on Windows, and specifically about a race condition in that code which has security implications for applications that use it.
Security issue in MSDN’s code example
Let’s have a look at MSDN’s example of a multithreaded pipe server implementation. Just looking at the _tmain function is enough.
- Create a pipe handle
- Wait for a client to connect
- If the client connected successfully, create a thread to process the client’s request and delegate to it the responsibility of closing the pipe handle when it’s done.
- Else, close the pipe handle
- Go back to step 1
Have you spotted the race condition? In this implementation, the pipe server will be unavailable in two cases:
- Between step 4 (line 74) and step 1 (line 27).
- Immediately after the new thread has closed the pipe handle (line 182) and until the main thread reaches step 1 (line 27, assuming the new thread was fast enough to close it before the main thread reached step 1).
In these cases, for a very brief time, our pipe server ceases to exist – there is no pipe server because all handles to it have been closed. Obviously, during that time, clients won’t be able to connect to the server. Albeit unwanted and warrants a fix, this isn’t the main concern here.
What makes this dangerous is the fact that a malicious user may hijack our pipe server by successfully timing the creation of a pipe server with the same name. Doing so allows the attacker to create the server with its own custom security attributes, which enables him to perform a DoS attack on the process running the pipe server and/or hijack connections to the pipe server and steal information sent by the clients. In addition to that, once the attacker has clients connecting to it, it might also be able to (depending on the client’s implementation) impersonate the client’s security context by calling ImpersonateNamedPipeClient.
Fixing the issue
One open handle at all times
Fixing this issue is just a matter of always having at least one handle open throughout the pipe server’s lifetime, i.e. making sure we don’t close the current handle before we create a new one. This guarantees that the next calls to CreateNamedPipe will use the same security attributes that were specified in our first call.
Here’s a fixed version of the code – the fix is in lines 39-40 and 63-64.
Going the extra mile
OK, so the issue is fixed. We could stop here, but I thought I’d mention a few more things we can do to make our application more secure.
- Pass FILE_FLAG_FIRST_PIPE_INSTANCE to the first call of CreateNamedPipe. Doing so will allow you to know whether someone (possibly an attacker) already owns the named pipe, and act accordingly.
- If you don’t use the impersonation features provided by APIs like ImpersonateNamedPipeClient, have your clients pass SECURITY_IDENTIFICATION (or whatever suits you) to CreateFile when connecting to the server.
- Consider passing PIPE_REJECT_REMOTE_CLIENTS to CreateNamedPipe if you’re not expecting any remote clients to connect to your pipe server.
- Hopefully, this goes without saying, but use security attributes that give users the minimal set of permissions they need in order to work with the pipe.
Premade code snippets are nice and easy to use, but should never be taken as gospel – especially since these snippets usually only treat the general case and rarely give much thought to security – we just saw how dangerous it could be to use code from the internet, even from reliable sources, without reviewing it first.
I hope that this case study encourages you to do things better: only use online code snippets as the basis to what you’re going to write; make sure you properly understand all aspects of the code; and… have yourself and your peers review it.