Username left in qhUserIDCache after unregistration

[dexgs]

Description

A user on my server wanted to claim a name they had previously registered to a certificate on a device they are no longer using. For clarity, we will say that their current certificate is registered to name A and they want to claim name B, which was registered to another certificate.

I removed the registration for name B, and re-named their current registration to B, but the operation was not successful (their display name changed, but if they disconnected and re-connected, they would still have name A).

I checked the database and found:

• The registration for name B was correctly removed
• The user's current registration was still for name A

From reading ServerDB.cpp, it seems like the most likely explanation is that name B was somehow left in qhUserIDCache despite the registration being removed. This would explain why renaming the registration failed, as doing so first checks for an ID corresponding to the new name in qhUserIDCache and aborts the name is associated with a different user ID.

I restarted the Mumble server, and the problem was resolved. This is consistent with my suspicion that name was still in quhUserIDCache, rather than the problem being caused by something more permanent like a bad database.

I highly suspect that there is a code path which can result in a registration being deleted while leaving its name still in qhUserIDCache.

Steps to reproduce

I could not reproduce the bug. I tried removing another registration and claiming its name, but this completed successfully.

Mumble version

No response

Mumble component

Server

OS

Linux

Reproducible?

No

Additional information

No response

Relevant log output

No response

Screenshots

No response

[dexgs]

Okay, I did some more testing and I actually can reproduce the bug now:

The steps are as follows:

Register a user with name "aaaaa" (this is just an example, any name will work)

Attempt to re-name an existing registration to a name which differs from "aaaaa" only in terms of case, e.g. "aAaaa" or "AAAAA". This should not complete successfully.

Then, delete the registration for "aaaaa".

Existing registrations may be re-named to "aaaaa" successfully, but trying to re-name an existing registration to any of the names with different case ("aAaaa", "AAAAA") will fail.

I think the problem is that when a registration is removed, the actual name of the registration is correctly removed from the cache, but all the other entries created as a result of handling case-insensitive matching are not removed, causing the problem.

i.e. attempting to re-name a user "aAaaa" while "aaaaa" is still registered results in a call to getUserID for "aAaaa" which matches the other registration and results in "aAaaa" being mapped to that ID in the cache. When "aaaaa" is unregistered, "aaaaa" is removed from the cache, but "aAaaa" is not.

In summary: the problem is that entries get created in the qhUserIDCache for every case-insensitive match, but only the actual name entry gets removed when a registration is removed.

My suggestion to correct this is rather than creating multiple cache entries in qhUserIDCache for each case-insensitive matching, lowercase all names before looking them up/inserting them in the cache. I will try doing this myself if I can find the time.

Here is a link to the offending code: https://github.com/mumble-voip/mumble/blob/master/src/murmur/ServerDB.cpp#L1740C1-L1744C3

[Krzmbrzl]

My suggestion to correct this is rather than creating multiple cache entries in qhUserIDCache for each case-insensitive matching, lowercase all names before looking them up/inserting them in the cache.

Yes that indeed sound like the correct way to go 👍

I will try doing this myself if I can find the time.

Thanks for the offer. However, the DB code is currently in the process of being rewritten entirely so any modifications in the current code will only make my life harder with that due to merge conflicts. I will make sure to address this in #6263