Missing null termination in memcached_sasl_authenticate_connection

[Stefan Friesel]

memcached_sasl_authenticate_connection reads the list of supported mechanisms from the server (PROTOCOL_BINARY_CMD_SASL_LIST_MECHS) into an uninitialized buffer and then passes it to sasl_client_start which expects a null-terminated string.

In my concrete scenario the server returns
header.response = {magic = 129, opcode = 32, keylen = 0, extlen = 0, datatype = 0, status = 0, bodylen = 5, opaque = 512, cas = 0}
with the body containing "PLAIN" (no null termination)

which then ends up in the string passed to sasl as "PLAIN<random characters>"

As the buffer is stack allocated (in my case it sometimes contained lines from /etc/resolv.conf), this could be used to inject mechanisms in the list.

Tested with libmemcached-1.0.18 and cyrus-sasl-2.1.25 (relevant code paths are the same in latest version)

[Aloÿs Augustin (aloysaugustin)]

This is affecting us too. When using memcached as the session storage backend for PHP, this often results in the SASL authentication failing, which prevents the sessions from being saved. Note that this was only observed when configuring libmemcached without --enable-debug. The -fsanitize options that are added in debug mode hide the issue.
I attached the (naive) patch that we're using.

[Brian Aker (brianaker)]

Hi,

From the code:
memcached_return_t rc= memcached_response(server, mech, sizeof(mech), NULL);
  if (memcached_failed(rc))

memcached_response() adds NULL to strings if memcached_response() successful, otherwise the code follows the error path. The author of this report mentions:
libmemcached/response.cc:619
which is:
          if ((rc= memcached_safe_read(instance, buffer, bodylen)) != MEMCACHED_SUCCESS)
          {
            return MEMCACHED_UNKNOWN_READ_FAILURE;
          }

As you can see, an error is returned back to memcached_sasl as mentioned in the original report.

All that you will achieve with calling memset() in this manner is hide any real bug that valgrind would find.

The reason why "This bug is difficult to reproduce since it depends on the contents of the stack.", is because there is no bug in the reporters work.

I cannot find a case of this bug being reported upstream, otherwise I would close it there with above message.

Thanks,

   -- Brian

FWIW I appreciate people going to the effort of reporting bugs, it takes time to write them up.