# grep -r '\.menu-cached' menu-cache-0.3.2 menu-cache-0.3.2/menu-cache-daemon/menu-cached.c: g_snprintf( buf, len, "/tmp/.menu-cached-%s-%s", dpy ? dpy : ":0", g_get_user_name() ); menu-cache-0.3.2/libmenu-cache/menu-cache.c: g_snprintf( buf, len, "/tmp/.menu-cached-%s-%s", dpy ? dpy : ":0", g_get_user_name() ); This can be easily fixed by using g_get_tmp_dir() instead of "/tmp".
Documentation for g_get_tmp_dir(): "Gets the directory to use for temporary files. This is found from inspecting the environment variables TMPDIR, TMP, and TEMP in that order. If none of those are defined "/tmp" is returned on UNIX…" That wouldn't change anything about the name being predictable. The code also calls unlink() on the to-be-created socket before binding it, avoiding potential symlink issues. So, can you elaborate on what you think is the issue here?
(In reply to comment #1) > That wouldn't change anything about the name being predictable. No, but other users will not be able to create a file with the same name in the directory (e.g., pam_mktemp sets TMPDIR to /tmp/.private/${USER}). > The code also calls unlink() on the to-be-created socket before binding it, > avoiding potential symlink issues. Not really -- there is a race condition there. The unlink() doesn't help the security issue anyway, since the client will connect to the socket regardless. > So, can you elaborate on what you think is the issue here? This is a classic Unix security vulnerability: http://siber.cankaya.edu.tr/ozdogan/SystemsProgramming/ceng425/node201.html
(In reply to comment #2) > (In reply to comment #1) > > That wouldn't change anything about the name being predictable. > > No, but other users will not be able to create a file with the same name in > the directory (e.g., pam_mktemp sets TMPDIR to /tmp/.private/${USER}). That's nice-to-have, but nothing I'd call vulnerability. > > > The code also calls unlink() on the to-be-created socket before binding it, > > avoiding potential symlink issues. > > Not really -- there is a race condition there. The unlink() doesn't help the > security issue anyway, since the client will connect to the socket > regardless. Yes, there is a race between unlink() and the following bind() and the unlink is of course not a remedy (as I might have wrongly implied in my previous comment). But what happens? Either the daemon creates the socket and all goes well or the attacker does, doing what? Contrary to the example you linked, this isn't a regular file that is written to. If the socket file exists, bind() fails (contrary to a default open()), and the daemon exits. I think you should talk to upstream about this. Should they see the need to change anything here, make another comment here and we'll be assigning this bug to the maintainers. Thanks.
(In reply to comment #3) > Either the daemon creates the socket > and all goes well or the attacker does, doing what? > Contrary to the example you linked, this isn't a regular file that is > written to. If the socket file exists, bind() fails (contrary to a default > open()), and the daemon exits. As I wrote: "The unlink() doesn't help the security issue anyway, since the client will connect to the socket regardless." bind() fails in the daemon, but the client application will connect to attacker's socket. > I think you should talk to upstream about this. I will file a bug, but the LXDE project is quite unresponsive.
Reported upstream: https://sourceforge.net/tracker/?func=detail&aid=3501347&group_id=180858&atid=894869
(In reply to comment #4) > (In reply to comment #3) > > Either the daemon creates the socket > > and all goes well or the attacker does, doing what? > > Contrary to the example you linked, this isn't a regular file that is > > written to. If the socket file exists, bind() fails (contrary to a default > > open()), and the daemon exits. > > As I wrote: "The unlink() doesn't help the security issue anyway, since the > client will connect to the socket regardless." bind() fails in the daemon, > but the client application will connect to attacker's socket. Ah, now we're on the same page. Yeah, the client could connect to such a socket, but it would require vulnerabilities in the protocol parsing to exploit it further. Your suggested g_get_tmp_dir change can indeed be used to improve security, this isn't a vulnerability on its own though.
(In reply to comment #6) > Your suggested g_get_tmp_dir change can indeed be used to improve security, > this isn't a vulnerability on its own though. Well, it's an easy local DoS, at least. Logging out of X, recreating the socket owned by another user and with no access permissions to the original user, and logging to X again, results in empty menu in lxpanel. These are the messages: ** ERROR **: Couldn't remove previous socket file /tmp/.menu-cached-:0-anon Unable to connect unable to connect to menu-cached.
FWIW, I don't see any obvious vulnerability with handling server data in menu-cache.c (on_server_io()). Although if(0 == memcmp(cache->md5, menu_cache_id, 32)) can potentially crash the client if menu_cache_id has insufficient length. But whoever wrote that code at least tried to handle arbitrary data correctly.
(In reply to comment #5) > Reported upstream: > https://sourceforge.net/tracker/?func=detail&aid=3501347&group_id=180858&atid=894869 Should be fixed in menu-cache-0.4 according to upstream.
