Issue in UdpTransport Handling of Malformed Packets
I see a potential issue in the UdpTransport class, where the threading model used to handle incoming UDP packets does not gracefully manage exceptions caused by malformed packets. This can lead to the silent failure of the thread responsible for receiving packets, while the main spp server application (For example any of the 4 examples) appears to continue operating normally while no longer being able to receive any packets.
Technical Detail: The UdpTransport class utilizes Python's threading library to spawn a separate thread for listening to incoming UDP packets. This thread is essential as it processes all incoming SPP packets. However, if a malformed packet is received that leads to an exception (e.g., due to a mismatch in expected packet size or format), the thread will terminate. The SPP server (e.g., sink.py, waypoint.py, remote.py, local.py) will continue running, unaware that it is no longer capable of receiving any further SPP packets.
Recreate issue: The issue mentioned above can be recreated by simply launching the example scripts (e.g., sink.py, waypoint.py, remote.py, local.py) as intended and sending some packets back and forth to confirm the working setup. Following this, send a malformed SPP packet (For example a packet_length and actual packet length mismatch) to one of the 4 ports specified in the example scripts. The terminal output should look similar to that shown below. As seen, the script (in this case sink.py) keeps running, but will no longer process any packets send from remote.py or local.py as the UDP input thread has died.
(base) xxx@xxx:~/Documents/python-spp/examples/udp_transport$ python sink.py
Running. Press <Enter> to stop...
Received Space Packet: <spp.space_packet.SpacePacket object at 0x7fe1c9101280>
packet_type: 1, packet_sec_hdr_flag: 0, apid: 333, sequence_flags: 3, packet_sequence_count: 1, packet_version: 0, packet_data_field: b'This is a test packet!'
Received Space Packet: <spp.space_packet.SpacePacket object at 0x7fe1c9101280>
packet_type: 1, packet_sec_hdr_flag: 0, apid: 333, sequence_flags: 3, packet_sequence_count: 1, packet_version: 0, packet_data_field: b'This is a test packet!'
Received Space Packet: <spp.space_packet.SpacePacket object at 0x7fe1c9101280>
packet_type: 1, packet_sec_hdr_flag: 0, apid: 111, sequence_flags: 3, packet_sequence_count: 1, packet_version: 0, packet_data_field: b'This is a test packet!'
Received Space Packet: <spp.space_packet.SpacePacket object at 0x7fe1c9101280>
packet_type: 1, packet_sec_hdr_flag: 0, apid: 111, sequence_flags: 3, packet_sequence_count: 1, packet_version: 0, packet_data_field: b'This is a test packet!'
Exception in thread Thread-1:
Traceback (most recent call last):
File "/home/xxx/anaconda3/lib/python3.9/threading.py", line 973, in _bootstrap_inner
self.run()
File "/home/xxx/anaconda3/lib/python3.9/threading.py", line 910, in run
self._target(*self._args, **self._kwargs)
File "/home/xxx/anaconda3/lib/python3.9/site-packages/spp/transport/udp.py", line 53, in _incoming_pdu_handler
self.indication(pdu)
File "/home/xxx/anaconda3/lib/python3.9/site-packages/spp/core.py", line 19, in _pdu_received
space_packet = SpacePacket.decode(pdu)
File "/home/xxx/anaconda3/lib/python3.9/site-packages/spp/space_packet.py", line 60, in decode
raise ValueError("Packet data length mismatch")
ValueError: Packet data length mismatch
Potential Impact: Given the nature of this project and the desire to emulate a space environment, this might lead to resiliency issues when used in a 'space' mission. Space missions often face harsh communication environments where packets can get corrupted due to physical signal degradation. Moreover, a malicious actor could exploit this vulnerability to induce a denial of service (DoS) by sending malformed packets, effectively disabling the command and control communication handled via SPP.
Proposed Solution: To ensure the continuous operation and resilience of the UDP transport mechanism, I propose wrapping the packet handling logic (inside _incoming_pdu_handler) within a try-except block, thus allowing the thread to recover and continue listening even after encountering malformed packets. I kept the error output to the terminal as before:
def _incoming_pdu_handler(self):
thread = threading.currentThread()
buffer_size = 10 * self.maximum_packet_length
while not thread.kill:
try:
pdu, addr = self._socket.recvfrom(buffer_size)
if thread.kill:
break
# forward-route the received pdu
if self.routing and addr in self.routing:
for dest in self.routing[addr]:
self._socket.sendto(pdu, dest)
self.indication(pdu)
except Exception as e:
print(f"Exception in CustomUdpTransport._incoming_pdu_handler: {e} from {addr}")
print(traceback.format_exc())
continue # Continue listening even after an exception