Address/port whitelist for gm_socket

[thegrb93]

Using the module in clientside is pretty dangerous since it allows servers you connect to access to your entire network. A text file in lua/bin containing safe addresses and ports would be a good security improvement.

Use normal behavior if the whitelist file is absent.

[thegrb93]

Proof of concept done, and I can finish it, but will take a little while cuz I'm unfamiliar with lua manipulation from C.

#include <iostream>
#include <regex>
#include <vector>

const std::wstring whitelist_str =
L"blah\\.org 80\r\n"
L"127\\.0\\.0\\.1 27015\r\n"
L"http\\://www\\.google\\.com.* 80\r\n";


std::vector<std::pair<std::wregex, unsigned short> > whitelist;

bool isSafe(const std::wstring& query, unsigned short port)
{
    for (auto i = whitelist.begin(), end = whitelist.end(); i != end; ++i)
    {
        if (std::regex_match(query.begin(), query.end(), i->first) && port == i->second)
        {
            return true;
        }
    }
    return false;
}

int main()
{
    std::wregex line_parser(L"(.+?)[\t ]+(\\d+)[\r\n]*");
    for (std::wsregex_iterator match = std::wsregex_iterator(whitelist_str.begin(), whitelist_str.end(), line_parser), end = std::wsregex_iterator(); match != end; ++match)
    {
        std::wcout << "Address: " << match->operator[](1) << "    Port: " << match->operator[](2) << std::endl;

        int port = _wtoi(match->operator[](2).str().c_str());
        if (port > 0 && port < 65536)
        {
            try
            {
                whitelist.push_back(std::pair<std::wregex, unsigned short>(match->operator[](1).str(), (unsigned short)port));
            }
            catch (std::regex_error)
            {
            }
        }
    }

    std::vector<std::pair<std::wstring, unsigned short>> tests = {
        {L"virus.com", 80},
        {L"192.168.1.15", 1000},
        {L"127.0.0.1", 27015},
        {L"http://www.google.com/hello", 80}
    };

    for (auto i = tests.begin(), end = tests.end(); i != end; ++i)
    {
        std::wcout << i->first << " port " << i->second << " is safe: " << isSafe(i->first, i->second) << std::endl;
    }

    return 0;
}

[danielga]

Beware that luasocket functions can be blocking. You should probably create a wrapper that forces non-blocking sockets and remove all related functions to change this. Regex is most likely overkill, since URL whitelisting is not possible, sockets deal with domains and IP addresses, that's it. I really don't want to do this whitelist in native code but got a few ideas on how to proceed, without making luasocket patches.

[thegrb93]

Thanks for the warning. Players will only be able to use luasocket on their own starfalls so they'll be at their own risk, but I'll add a note about that in the documentation. Yeah, you're right, totally forgot url junk is http specific so regex isn't needed.

[thegrb93]

It just occurred to me that I made this issue specific to the clientside build, but the serverside would also probably want this in case an addon tries to be malicious.

[thegrb93]

I was looking at https://stackoverflow.com/questions/19023018/overriding-c-library-functions-calling-original and seems like you could just wrap getaddrinfo with the --wrap linker arg.

[thegrb93]

https://stackoverflow.com/questions/12172988/in-c-is-it-possible-to-change-exported-function-name-to-different-one/12173140#12173140

This link contains info how to use --wrap

[danielga]

That only works for GCC/Clang (maybe) and it's not a great solution overall. My idea was just to load luasocket components as usual but, right after, get the real functions and make native wrappers that check the input, overriding the exposed functions with these.

[danielga]

On second thought, this won't work for meta methods, since those are not exported.

[thegrb93]

Shit, yea.

Maybe for the luasocket sources you could define getaddrinfo as __wrap_getaddrinfo then in the project settings.

[thegrb93]

Or just define getaddrinfo as __wrap_getaddrinfo for all sources and add an #undef getaddrinfo where you want to call the original.

[thegrb93]

Kinda dirty tho, ye.