oremanj/python-netfilterqueue

__str__ method is not safe

elqver opened this issue · 2 comments

This code results with segmentation fault

from typing import List

import netfilterqueue
from netfilterqueue import NetfilterQueue


packet_storage: List[netfilterqueue.Packet] = []


def print_packets():
    for packet in packet_storage:
        print(packet)


def handle_packet(packet: netfilterqueue.Packet):
    packet.retain()
    packet.accept()
    packet_storage.append(packet)
    print_packets()


q = NetfilterQueue()
q.bind(11, handle_packet)
q.run()

Looks like str method is not safe, because there is no check for NULL of self.payload:

    def __str__(self):
        cdef iphdr *hdr = <iphdr*>self.payload
        protocol = PROTOCOLS.get(hdr.protocol, "Unknown protocol")
        return "%s packet, %s bytes" % (protocol, self.payload_len)

Is this ok, can that be fixed?

self.payload is a defined as a pointer and passed to a C lib function (libnetfilterqueue). the payload data is allocated in memory by the C lib and initialized to point to the data. Because of this, once you set a verdict on the packet the payload and all other pointers/references held to memory allocated in the C lib will no longer be valid.

@DOWRIGHTTV your understanding of the libnetfilter_queue API is incorrect as I explained in the other issue you opened; the payload pointer points into a buffer stack-allocated in run() and we null it out once it might have been reused for a different packet.

@elqver you're quite right, this is my bad and I just uploaded #89 to fix it!