T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/minecraft-status.py:31
- Finding
- Unbounded parsing of attacker-controlled protocol response lengths<![CDATA[ ## Vulnerability Details **File Location**: `scripts/minecraft-status.py`, lines 31-43 and 67-80 **Vulnerability Type**: Improper input validation and uncontrolled resource consumption **Risk Level**: Medium ### Vulnerable Code ```python def decode_varint(sock): """Decode a Minecraft varint from socket.""" num = 0 shift = 0 while True: byte = sock.recv(1) if not byte: raise IOError("Connection closed") byte = byte[0] num |= (byte & 0x7F) << shift if not (byte & 0x80): break shift += 7 return num ``` ```python def read_status_response(sock): """Read server status response.""" # Packet length length = decode_varint(sock) # Packet ID should be 0x00 packet_id = decode_varint(sock) # Response length response_len = decode_varint(sock) # Response data (JSON) response = b"" while len(response) < response_len: response += sock.recv(response_len - len(response)) return json.loads(response.decode('utf-8')) ``` ### Technical Analysis The parser trusts length fields supplied by the remote Minecraft server. `decode_varint()` does not enforce Minecraft's five-byte maximum for a 32-bit VarInt. A server can therefore continuously send bytes with the continuation bit set, causing the integer and shift count to grow while consuming CPU. More critically, `response_len` is used without any upper bound to control how much response data is read and retained. Repeated immutable byte-string concatenation can also result in excessive copying and poor memory performance. The response loop does not check whether `sock.recv()` returns `b""`. A clean connection closure before the declared body length is received consequently causes the loop to repeatedly append an empty byte string without making progress. Because reads from an already closed socket continue returning `b""`, the configured socket timeout does not necessarily termin ...[truncated 1412 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Reject VarInts longer than five bytes, as required for Minecraft 32-bit VarInts. - Define a conservative maximum packet and JSON response size, such as 1 MiB, and reject zero, negative, inconsistent, or oversized lengths. - Detect `recv()` returning `b""` and immediately raise a connection-closed exception. - Read data through a bounded helper that guarantees either exactly the requested number of bytes or a controlled failure. - Accumulate chunks in a list or `bytearray` and join once, rather than repeatedly concatenating immutable byte strings. - Validate the packet ID and ensure the declared packet length is consistent with the contained fields. - Close the socket through a context manager or `finally` block so malformed responses cannot leave resources open. Example hardening pattern: ```python MAX_VARINT_BYTES = 5 MAX_RESPONSE_SIZE = 1024 * 1024 def decode_varint(sock): value = 0 for index in range(MAX_VARINT_BYTES): raw = sock.recv(1) if not raw: raise IOError("Connection closed while reading VarInt") byte = raw[0] value |= (byte & 0x7F) << (7 * index) if not byte & 0x80: return value raise ValueError("VarInt exceeds five bytes") def recv_exact(sock, size): if size < 0 or size > MAX_RESPONSE_SIZE: raise ValueError("Invalid response length") chunks = [] remaining = size while remaining: chunk = sock.recv(min(remaining, 65536)) if not chunk: raise IOError("Connection closed before response completed") chunks.append(chunk) remaining -= len(chunk) return b"".join(chunks) ``` ]]>