oremanj / python-netfilterqueue

Python bindings for libnetfilter_queue
MIT License
248 stars 96 forks source link

__str__ method is not safe #83

Closed elqver closed 2 years ago

elqver commented 2 years ago

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?

DOWRIGHTTV commented 2 years ago

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.

oremanj commented 2 years ago

@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!