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.
Let’s have a look at MSDN’s example of a multithreaded pipe server implementation. Just looking at the _tmain function is enough.
Have you spotted the race condition? In this implementation, the pipe server will be unavailable in two cases:
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 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.
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.
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.