ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.72k stars 625 forks source link

[GoldSource] SV_ParseResourceList crash server #2143

Open SkillartzHD opened 5 years ago

SkillartzHD commented 5 years ago

If the attacker overpowered SV_ParseResourceList with many custom resources then the server causes a crash, this can be done by a memory hack to the client or creating a bot software to attack the server, normal on the client is only one request

The solution is to add this line in SV_ParseResourceList

    int msgshort;
    msgshort = MSG_ReadShort();
    if (msgshort > 1){
        SV_DropClient(host_client, false, "SV_ResoucesList : Invalid custom resources 1/%d", msgshort);
        return;
    }
Splatt581 commented 5 years ago

Game client can actually do a DoS attack on the server if it sends clc_resourcelist with a large number of invalid resources. For example, there is an exploit that sends the following list of resources:

0000 0010:  05 88 13 54 45 53 54 00 02 00 00 ff ff ff ff 00  ...TEST. ........
0000 0020:  54 45 53 54 00 02 01 00 ff ff ff ff 00 54 45 53  TEST.... .....TES
0000 0030:  54 00 02 02 00 ff ff ff ff 00 54 45 53 54 00 02  T....... ..TEST..
0000 0040:  03 00 ff ff ff ff 00 54 45 53 54 00 02 04 00 ff  .......T EST.....
0000 0050:  ff ff ff 00 54 45 53 54 00 02 05 00 ff ff ff ff  ....TEST ........
0000 0060:  00 54 45 53 54 00 02 06 00 ff ff ff ff 00 54 45  .TEST... ......TE
0000 0070:  53 54 00 02 07 00 ff ff ff ff 00 54 45 53 54 00  ST...... ...TEST.
0000 0080:  02 08 00 //this is a small part of the packet since it is very large

The server freezes for a long time with the error Ignoring invalid custom decal from %s:

bov2

Now lets go to the contents of packet: 05 - id of clc_resourcelist message; 88 13 - number of resources in list (5000). In real conditions, the game client either does not transfer resources at all, or it transfers one resource in this list - tempdecal.wad logo; 54 45 53 54 00 - current resource name (TEST); 02 - unused resource type (t_model). In real conditions, game client uses t_decal (0x03); 00 00 - resource number in the list; ff ff ff ff - invalid resource size (-1), which is why an error occurs. 00 - additional flags. etc..

Probably the most correct solution would be to check the number of resources in the list in SV_ParseResourceList and then disconnect game client if there is more than one resource in the list. Source: https://github.com/dreamstalker/rehlds/blob/65c6ce593b5eabf13e92b03352e4b429d0d797b0/rehlds/engine/sv_upld.cpp#L395-L401

@mikela-valve I can provide an exploit that using this vulnerability if it is needed, because it cannot be done through a pure game client.

metita commented 5 years ago

@mikela-valve This should be considered for a future release, security updates should be high-priority.

Splatt581 commented 1 month ago

This vulnerability was fixed in the GoldSrc Anniversary Update. Now the number of resources in clc_resourcelist is controlled by the sv_uploadmaxcount cvar on the server side. I recommend setting the cvar to 1 to completely prevent this attack method, because the original game client can't send more than one custom resource (logo decal) anyway.

So I think the issue can be closed.