ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.69k stars 623 forks source link

Invalid player name can be used to attack servers and cause 100% CPU utilisation #2421

Closed SamVanheer closed 5 years ago

SamVanheer commented 5 years ago

This issue has been reported a few times on the ReHLDS issue tracker: https://github.com/dreamstalker/rehlds/pull/595 https://github.com/dreamstalker/rehlds/issues/629 https://github.com/dreamstalker/rehlds/issues/631 https://github.com/dreamstalker/rehlds/issues/635 https://github.com/dreamstalker/rehlds/issues/642

Essentially an invalid name can cause an infinite loop in SV_CheckForDuplicateNames that can freeze the server or at least keep it at maximum CPU utilization.

SamVanheer commented 5 years ago

@mikela-valve This is a high priority issue since it can be used to render servers unusable.

mikela-valve commented 5 years ago

It looks like this is due to Info_SetValueForStarKey not removing all instances of a key from the userinfo when setting a new value. The checks for invalid characters already exist in SV_CheckForDuplicateNames and bad names are filtered out of it, but when it goes to update the name in the userinfo to 'unnamed' it only removes the first 'name' field that it finds, leaving any additional ones that have invalid values.

SkillartzHD commented 5 years ago

This is already fixed in 7559 build

mikela-valve commented 5 years ago

@SkillartzHD I think you might be referring to the nickname filtering that is indeed already present in the current release, but the issue here is actually that you can get around the filtering by inserting duplicate invalid name fields in the infostring when connecting. This causes the server to fail trying to remove the offending name field as it doesn't remove the duplicate fields from the infostring, only the first one it finds.

SkillartzHD commented 5 years ago

@mikela-valve He currently manages to make that replace in Infostring, but connects 2 players with the same name (that would be the only problem), but that infamous loop that existed under version 6153 no longer exists, I do not understand one thing why there is a "console" exist in the blacklist for sv_checkforduplicatenames ?

**public static string RandomSV_CheckForDuplicateNamesBug2(int length)
{
    const string chars = "ƒ†‡‰•˜™šœžŸ¡¢£¤¥¦§©«¬®°±²³µ¶üû";
    return new string(Enumerable.Repeat(chars, length).Select(s => s[random.Next(s.Length)]).ToArray());
}**

`**

            string SV_CheckForDuplicateNames_method1 = "\"";
            string SV_CheckForDuplicateNames_method2 = "."+RandomSV_CheckForDuplicateNamesBug2(1)+".";

            Random ra = new Random();
            int r1 = ra.Next(1, 1024);
            int r2 = ra.Next(0, 2);
            int r3 = ra.Next(0, 1);
            int r4 = ra.Next(0, 1);
            int r5 = ra.Next(0, 300);
            int r6 = ra.Next(1, 20);
            int r7 = ra.Next(20, 150);
            int r8 = ra.Next(0, 1024);
            int r9 = ra.Next(3500, 50000);
            int r91 = ra.Next(60000 - 30000);

            var modelsn = new List<string> { "arctic", "gign","gsg9", "leet", "sas", "terror","urban","vip", "guerilla", };

            int index = random.Next(modelsn.Count);
            var model = modelsn[index];
            modelsn.RemoveAt(index);

            string userkey = "\\prot\\3\\unique\\-1\\raw\\steam\\cdkey\\" + CDKeyGenerator(32)+ " ";
            string userinfo = "\\name\\" + NameText.Lines[0] + SV_CheckForDuplicateNames_method1 + "\\_cl_autowepswitch\\" + r3 + "\\bottomcolor\\" + r7 + "\\cl_dlmax\\" + r8 + "\\cl_lc\\" + r3 + "\\cl_lw\\" + r3 + "\\cl_updaterate\\" + r6 + "\\model\\" + model + "\\topcolor\\" + r5 + "\\_vgui_menus\\" + r2 + "\\rate\\" + r9 + "\"";
            string userinfo2 = "\\name\\" + NameText.Lines[0] + SV_CheckForDuplicateNames_method2 + "\\_cl_autowepswitch\\" + r3 + "\\bottomcolor\\" + r7 + "\\cl_dlmax\\" + r8 + "\\cl_lc\\" + r3 + "\\cl_lw\\" + r3 + "\\cl_updaterate\\" + r6 + "\\model\\" + model + "\\topcolor\\" + r5 + "\\_vgui_menus\\" + r2 + "\\rate\\" + r9;**`
Splatt581 commented 5 years ago

@SkillartzHD

This is because game server checks renamed and malformed nicknames for a duplicate, but displays next malformed nicknames, for example, unnamed and asd...

But if you try to set 5-6 identical malformed nicknames, like that \name\asd..\name\asd..\name\asd..\name\asd..\name\asd..\, and make 2 full connections, server will hang. In general, when connecting, game server renames malformed nicknames several times, but in the end it will have to check two identical malformed nicknames.

mikela-valve commented 5 years ago

Fixed in beta 'Exe build: 11:12:36 May 21 2019 (8244)'.

Splatt581 commented 5 years ago

I confirm, fixed. Now all duplicate nicknames in userinfo are deleted, except very first one.

mikela-valve commented 5 years ago

Closing as fixed.