Skip to content

__str__ method is not safe #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
elqver opened this issue Feb 8, 2022 · 2 comments · Fixed by #89
Closed

__str__ method is not safe #83

elqver opened this issue Feb 8, 2022 · 2 comments · Fixed by #89

Comments

@elqver
Copy link

elqver commented Feb 8, 2022

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
Copy link

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 oremanj mentioned this issue Apr 19, 2022
Merged
@oremanj
Copy link
Owner

oremanj commented Apr 19, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check whether payload is NULL before accessing it in __str__ oremanj/python-netfilterqueue
3 participants
@oremanj @elqver @DOWRIGHTTV