Potential buffer overflow in demoplayer module of GoldSource Engine

Introduction Hey. There's a potential vulnerability in the **GoldSource Engine** that allows to write data to stack of arbitrary size, thereby causing a buffer overflow and the ability to execute assembler code using **.dem** files. **Description** The problem is located in the **DemoPlayer::ReadDemoMessage** function, which lies in **demoplayer** module, which is called when the **viewdemo** console command is executed. Since this command is not included in the filter list of the **ValidStuffText** function, and the .dem extension is not included in the filter list of the **IsSafeFileToDownload** function, the server can upload the malformed dem-file to the client and run it. Also, it is possible for hackers to spread incorrect dem-files without using HLDS, sending a malformed dem to users on web-resources or using messengers. The following opcodes are vulnerable: **DemoCmd::PlaySound** (8), **DemoCmd::PayLoad** (9). These opcodes contain a variable responsible for the amount of data that must be read by the **BitBuffer::ReadBuf** function. Since the variable where the read result fits is located in the stack memory, it is possible to overflow the buffer and overwrite the return address on its own code. At the moment, the vulnerability does not pose a risk, since the limit of any network packet is 4010 (**MAX_UDP_PACKET** constant), which excludes the possibility of a buffer overflow in the DemoCmd::PayLoad command (which is 32767 bytes in size). At the same time, the buffer for DemoCmd::PlaySound is 256 bytes, but this buffer is located in front of the buffer for DemoCmd::PayLoad, which will not allow rewriting the return address on the stack. However, there is a potential danger that in future recompiling of the project, the buffers may be swapped, which will cause a vulnerability for the DemoCmd::PlaySound package. **Possible Solutions** Create conditions that don't allow to read too much data. For example: ``` size = BitBuffer::ReadLong(buf); if (size > sizeof(buffer)) size = sizeof(buffer); BitBuffer::ReadBuf(buf, size, buffer); ``` Impact ------- As said before, an attacker can remotely execute arbitrary assembly code on the clients or player can run malformed dem-file himself with **viewdemo** console command.