search

SWI-Prolog / packages-clib

Assorted external libraries: processes, sockets, MIME, CGI, etc.
8 stars 19 forks source link

Connecting to the prolog_server from a machine without a hostname causes it to fail #47

Closed thetrime closed 1 year ago

thetrime commented 1 year ago

server_loop/2 has this code:

server_loop(ServerSocket, Options) :-
    tcp_accept(ServerSocket, Slave, Peer),
    tcp_open_socket(Slave, InStream, OutStream),
    set_stream(InStream, close_on_abort(false)),
    set_stream(OutStream, close_on_abort(false)),
    tcp_host_to_address(Host, Peer),
    (   Postfix = []
    ;   between(2, 1000, Num),
        Postfix = [-, Num]
    ),
    ...

If the machine making the connection has an IP address which the server cannot resolve, tcp_host_to_address/2 raises an exception and the thread dies. The Host variables is only used to make an alias for the handling thread, so it seems reasonable to catch this exception and substitute the IP directly if it cannot be resolved.

JanWielemaker commented 1 year ago

I'm happy with that. Although I wonder whether one wants to accept such dubious connections .... If you want some more security, use the ssh server from the libssh add-on. Do you make a PR?

thetrime commented 1 year ago

I had to do some testing first :)

We definitely don't want to accept such dubious connections. The allow/2 will fail unless you explicitly list the IP of the connections you'd like to accept. The problem is that we don't even get that far - when forming the alias of the thread to handle the connection, the server loop itself dies.

thetrime commented 1 year ago

(The problem being you can just DOS a prolog server by connecting to the port from, say, your phone. One connection and the server stops running). I'd love to use the libssh add-on, but unfortunately I doubt I'll be able to convince people to upgrade far enough to get it :(

JanWielemaker commented 1 year ago

The DoS attach is a good reason. The libssh add-on may run on rather old versions. Not sure. It may depend on some more recent fixes and additions to controlling the toplevel. It doesn't require recent stuff that probably does affect your code base such as the native support for rationals.

Possibly you need your old patch unless you kept packages/clib from the current development tree ...