How to sanitize user inputs from untrusted sources?

[jjtolton]

My expectation is the shared library functionality will be very popular (for some definition of "very popular") in web development circles, as it will function both in the browser via WASM and server via shared library.

Until we have a proper C-API for constructing values, we are currently limited to providing strings to the Scryer process.

For some users, it would be very easy to do something like:

with scryer_process() as wam:
   results = wam.eval(f"input_searchresults('{user_input}', Results).")

perhaps not realizing they are inviting remote code execution or cross site scripting vulnerabilities. It would be good to advise what the correct way of reading user input is -- I'm not even sure myself!

[hurufu]

If Scryer runs as a library then conceptually speaking the caller is responsible for doing string sanitization. I see at least 2 types of possible problems:

1. What if user_input equals to "hello', Results); bad_predicate('world" – then Prolog doesn't have any chance to capture this situation, thus it must be handled in Python.
2. When user_input contains otherwise correct but logically wrong data, like you are expecting a number but it contains a non-numeric string – then it should be checked and handled as usual inside input_searchresults/2 predicate.

[jjtolton]

Well I don't know what the "best" solution is, but I was thinking that maybe there's a solution that is better than writing to disk.

import os
import time
pipe_name = '/tmp/testpipe'

if __name__ == '__main__':
    if not os.path.exists(pipe_name):
        os.mkfifo(pipe_name)
    with open(pipe_name, 'wb') as f:
        f.write(b'hello from writer!\n')
        print("sleeping...")
        time.sleep(3)
    print('done!')

:- use_module(library(charsio)).
:- use_module(library(iso_ext)).
:- use_module(library(dcgs)).
:- use_module(library(pio)).

hello(hello) --> ..., "hello", ... .

?- open("/tmp/testpipe", read, Stream, [type(binary)]),
   get_line_to_chars(Stream, Chars, []),
   phrase(hello(Words), Chars).
%@    Stream = '$stream'(0x5613fc20b840), Chars = "hello from writer!\n ...", Words = hello
%@ ;  false.

This worked out rather nicely!

There is some nice stuff in charsio for reading and writing Prolog terms, so some combination of these will probably work well.

[jjtolton]

I was sad that it did not work with the pio library, but I might be using it incorrectly:

?- phrase_from_file(hello(World), "/tmp/testpipe", [type(binary)]).
%@    false.

?- open("/tmp/testpipe", read, Stream, [type(binary)]),
   phrase_from_stream(hello(World), Stream).
%@    false.

Maybe pio doesn't work with binary?

[hurufu]

library(pio) only works with the regular files and it doesn't work with pipes to the best of my knowledge.

[jjtolton]

Strange that it doesn't. It seems to use get_n_chars/3 here, and get_n_chars/3 works.

hello(hello) --> ..., "hello", ... .
n_chars(10).
?- open("/tmp/testpipe", read, Stream, [type(binary)]),
   n_chars(NChars),
   get_n_chars(Stream, NChars, Chars),
   phrase(hello(Words), Chars).
%@    Stream = '$stream'(0x55f0b163c810), NChars = 10, Chars = "hello from", Words = hello
%@ ;  false.

[hurufu]

Try to close your your pipe immediately after writing to it, maybe it is just cached by the OS.

[jjtolton]

I think reversing @bakaq 's algorithm in PR #2493 is probably the key to a convenient API that doesn't expose the user to string injection attacks. It might not be the best "Prolog" approach but from a shared library perspective it would be perfect, and it would be a good stepping stone to the C-API.

[bakaq]

That would be the "deserialization" part (in quotes because there is more to it than that). What I think would actually be necessary to use that as escaping is to have some way of turning a Value into something akin to what write_canonical/2 would print, because then you can really just interpolate it into a query string and it would just work. This doesn't seem all that difficult, but there are a lot of edge cases, and any string would get ~7 times bigger because of '.'(a,'.'(b,...)) shenanigans. Even better would be to have an API to put the term directly into the Scryer Prolog heap and let it handle the rest, but with what I know of the internals that seems to have even more tricky edge cases.

[hurufu]

A general remark. When you do the following:

wam.eval(f"input_searchresults('{user_input}', Results).")

Then f"input_searchresults('{user_input}', Results)." string contains an actual source code that is being executed. Do you copy random sources from the internet and compile them into your program? The same is here, such interface is insecure by design and can't be done secure. Treat that string as an actual sources and allow access to that string only to developers in your project.

If you want a secure interface then possibly some other approaches are needed as discussed here.

P.S. This problem isn't Prolog specific, you will have all exactly the same vulnerabilities if you would like for example embed JavaScript into your application.

[hurufu]

Also you can take some inspiration from GNU Prolog approach.

[jjtolton]

I think that will be very useful for API design!

[jjtolton]

P.S. This problem isn't Prolog specific, you will have all exactly the same vulnerabilities if you would like for example embed JavaScript into your application.

You're absolutely right, it's not Prolog specific. But it does happen all the time, especially with SQL.

I think many developers will be even less familiar with Prolog than SQL and would be tempted to do something like this out of frustration unless they are provided a copy/paste alternative to experiment with!

[hurufu]

I didn't try it myself, but maybe it is possible to use library(ffi) inside shared lib. It will look the following:

In Prolog define a native function:

:- use_module(library(ffi)).
main :-
    use_foreign_module("/path/to/myApp", [get_config([], cstr)]),
    ffi:get_config(UserConfig),
    do_stuff(UserConfig).

Then during startup you would just do:

wam.eval("main.")

and then in the myApp program you could have symbol get_config with the following signature get_config(void) -> cstr. And inside it's implementation you could actually provide a user defined data.

This will create a very narrow interface to feed user provided data into Prolog runtime.

[jjtolton]

Having progressed a little further with Prolog, the answer and suggested best practice now is fairly clear. Simply take a user input, then

1. string interpolate as you would naively (unsafe), then
2. use read_from_chars/3 to convert the string to Prolog terms
3. then feed the Prolog terms into a Prolog predicate or better yet a meta-interpreter
4. Under no circumstances use call/N on the result of bullet [1] or [2] and you will be fine.