cc-tweaked / CC-Tweaked

Just another ComputerCraft fork
https://tweaked.cc
932 stars 211 forks source link

[Real World Security] wget http://127.0.0.1 is blocked, but wget http://[::1] is not #594

Closed moritzuehling closed 3 years ago

moritzuehling commented 3 years ago

In short: image

[::1] is the IPv6 loopback interface, so it's essentially the IPv6 equivalent to 127.0.0.1

I'm using FTB Revelation, which is Minecraft 1.12.2, CC:Tweaked 1.84.0 (which was included in FTB modpack). I've tested this in Single Player mode.

If that's already fixed in newest version, feel free to close this!

Steps to reproduce:

In general, a security model that relies on blacklists will almost always have some problems, but I do get the intention. It might be worth considering to use some sort of preflight-requests (e.g. if you request http://example.com/test/asdf.lua, it triggers a preflight to http://example.com/test/computer-craft-lua-allowed.txt, which must return the string cc-allowed, or something like that.) This gives the server operator control over if they want to allow computer-craft.

This would also allow server-admins to host some specific scripts on 127.0.0.1 on the server, so the less experienced community members can enjoy them.

Quick additional question: is there a way to whitelist blacklisted IPs? For my singleplayer map I'd love it if I can just add an entry to my hosts file that resolves mc to 127.0.0.1, and then just wget run http://mc/test.lua for rapid testing. At the moment that entry points to ::1, but that might not work in the future ;) (For multiplayer that obviously won't work, but I already have ideas for that, that I just don't want to do right now)

I wish you all a great day!

Lupus590 commented 3 years ago

Check the config, the blacklist and whitelists are in there, 127.0.0.1 is just a default. I'm not sure if it will understand IPv6 addresses but it's worth trying.

moritzuehling commented 3 years ago

I made a couple of edits, sorry about that.

In general, I strongly believe that if 127.0.0.1 is blacklisted by default, so should ::1.

SquidDev commented 3 years ago

@Lupus590 The http rule system supports IPv6, but we've only got a rule for fd00::/8. We really should add the following:

Actually, scratch that. We'd probably need to add ipv4 and ipv6 blocks. Might be worth adding some special syntax for this, then we can just do InetAddress.isAnyLocalAddress.

For when I come top implement this, some test cases which should all fail:

print(http.checkURL("http://0.0.0.0"))
print(http.checkURL("http://localhost"))
print(http.checkURL("http://lvh.me"))
print(http.checkURL("http://127.0.0.1"))
print(http.checkURL("http://[::1]"))
print(http.checkURL("http://[::]"))
print(http.checkURL("http://172.17.0.1"))

print(http.checkURL("http://192.168.1.114"))
print(http.checkURL("http://[0:0:0:0:0:ffff:c0a8:172]"))
SquidDev commented 3 years ago

Quick additional question: is there a way to whitelist blacklisted IPs?

Should just be possible to remove the various lines from the computercraft.cfg file. If you clear the deny list, it should allow any domain.

This would also allow server-admins to host some specific scripts on 127.0.0.1 on the server, so the less experienced community members can enjoy them.

While you can obviously remove 127.0.0.1 from the block list, there's a couple of other work arounds:

In general, a security model that relies on blacklists will almost always have some problems.

Oh, for sure. However, I think it sits in the sweet spot between "entirely unsafe by default" (i.e. can hit any local server) and "far too restrictive by default" (old CC used to just allow pastebin.com). Well, it would if I'd actually configured the limited IPs correctly.

RGFTheCoder commented 3 years ago

The 10.0.0.x family also has to blocked for LAN on XFinity Networks

xAnavrins commented 3 years ago

fc00::/7 and fe80::/10 should probably be added to the default blacklist as they are ipv6 private ranges.

The 10.0.0.x family also has to blocked for LAN on XFinity Networks

All of RFC 1918 ranges are already blacklisted by default.

SquidDev commented 3 years ago

As mentioned above:

We'd probably need to add [the] ipv4 and ipv6 blocks [which includes 10.0.0.0/8, fc00::/7 and fe80::/10]. Might be worth adding some special syntax for this, then we can just do InetAddress.isAnyLocalAddress.

I'm wrong here - we actually need to use isSiteLocalAddress, isLinkLocalAddress, isLoopbackAddress and isAnyLocalAddress. Thanks Java.