demoinfocs-golang: Parser errors on encrypted net-message when decryption key is provided

Describe the bug

Hello!

Some Valve demos can lead to an error when parsing them with the new NetMessageDecryptionKey property added in the last version. The error occurs in the CSVCMsg_EncryptedData handler because it’s trying to read out of the bit reader boundaries.

To Reproduce

This zip archive contains 2 affected demos.

Code:

Parsing one of the attached demos using the snippet from the last release should trigger a panic.

Expected behavior

I think errors occurring while reading an encrypted data message should not prevent a demo parsing. It’s supposed to be private and may change in the future.

Library version 2.13.0

Additional context

I’m currently using my own implementation to handle encrypted messages with AdditionalNetMessageCreators but since this project now support it, I would like to use it and remove unnecessary code 😄

I looked into a possible solution but the gobitread package doesn’t expose a public function to know how many reading bytes are available which prevent me to ignore a corrupted message easily.

What I tried:

+++ b/pkg/demoinfocs/net_messages.go
@@ -98,6 +98,10 @@ func (p *parser) handleEncryptedData(msg *msg.CSVCMsg_EncryptedData) {
 	br := bit.NewSmallBitReader(r)
 
 	paddingBytes := br.ReadSingleByte()
+	messageSize := len(b)
+	if 1+int(paddingBytes) > messageSize {
+		return
+	}
 	br.Skip(int(paddingBytes) << 3)
 
 	bBytesWritten := br.ReadBytes(4)
@@ -106,8 +110,16 @@ func (p *parser) handleEncryptedData(msg *msg.CSVCMsg_EncryptedData) {
 	unassert.Same(len(b), 5+int(paddingBytes)+nBytesWritten)
 
 	cmd := br.ReadVarInt32()
+
+	if messageSize-nBytesWritten-int(paddingBytes)-5 != 0 {
+		return
+	}
+
 	size := br.ReadVarInt32()
 
+	if nBytesWritten-2 != int(size) {
+		return
+	}
 	m := p.netMessageForCmd(int(cmd))
 
 	if m == nil {

But it’s not enough for the demo match730_003449478367177343081_1946274414_112.dem, I think it would easier to know how many bytes are available before actually reading it to prevent an error or maybe you have an other idea?

If that can help, here is the code of my own handler, I’m checking if reading bytes is safe before actually doing it at each step: Note: reader is a GO bytes.Reader small wrapper, let me know if you need more details.

parser.RegisterNetMessageHandler(func(m *msg.CSVCMsg_EncryptedData) {
	isPublicKey := m.KeyType == encryptedKeyTypePublic
	if !isPublicKey || analyzer.iceKey == nil {
		return
	}

	messageSize := len(m.Encrypted)
	isMessageMalFormed := messageSize%analyzer.iceKey.BlockSize() != 0
	if isMessageMalFormed {
		return
	}

	plaintext := make([]byte, messageSize)
	analyzer.iceKey.DecryptFullArray(m.Encrypted, plaintext)

	reader := bytesreader.NewBytesReader(plaintext)
	paddingBytes := reader.ReadUInt8()
	if 1+paddingBytes > uint8(messageSize) {
		return
	}
	reader.Skip(int64(paddingBytes))
	if reader.Remaining() <= 4 {
		return
	}
	bytesWritten := reader.ReadInt32BE()
	if reader.Remaining() != int(bytesWritten) {
		return
	}

	cmd := reader.ReadVarInt32()
	if cmd == uint32(msg.SVC_Messages_svc_UserMessage) {
		size := reader.ReadVarInt32()
		if reader.Remaining() != int(size) {
			return
		}
		currentOffset := reader.CurrentOffset()
		userMessage := new(msg.CSVCMsg_UserMessage)
		proto.Unmarshal(plaintext[currentOffset:], userMessage)
		switch userMessage.MsgType {
		case int32(msg.ECstrike15UserMessages_CS_UM_SayText2):
                        //  Chat message here
		}
	}
})

Thank you!