Local IP blacklist bypass

[TheHackerDev]

Issue

It appears that we can bypass the local IP blacklist (implemented in #16) by replacing decimal characters with hex. I have tried with octal, and that didn't appear to work. Furthermore, it seems to pass some type of malformed request through when I use hex as well; this may be an issue with Gin, but I'm not 100% sure. EDIT: Turns out this was because I was trying to use HTTPS with a listener that didn't support it

BeePing listener:

HTTP listening service:

Curl requests:

Fix

My suggested fix is to cast the destination IP to an integer in the validateTarget() function, before parsing the IP with net.ParseIP(), because that function is unable to parse hex values. However, I want to be sure that we catch all test cases before doing so.

Other Notes

It's also worth tracking down what's happening with the data as it's passed through, as it appears to be corrupted or malformed somehow. When I debugged the request, the req value seemed fine (in the CheckHTTP() function), but there were two other weird values that probably shouldn't have been so off: EDIT: Ignore this, see edit above

Thanks to @jimen0 for bringing this to my attention. He may be able to chime in here as well.

Cheers,
Aaron (insp3ctre)

[jimen0]

Hello.

As Aaron and I were discussing through DMs, there are some points that we should think about.
First, SSRF is really hard to fix using blacklists, but I think that the idea of converting it to decimal number notation and doing the check is a really cool idea.

There are still a few scenarios that we should be concerned about:

• SSRF can be performed by creating a request to, for example, https://mylegitdomain.com and then, redirecting visitor to http://192.168.0.1:80/, if the software follows the redirect, the attack will take place

• Reserved IP addresses, for example, should be blocked by default. See this

• There are tons of notations for IP addresses and we can mix them (hex + decimal number, hex + decimal octets, etc), IMO we should do as many tests as possible for this and then fuzz it to check that we fixed as many scenarios as possible

Hope you keep up the work on this project. I think that it's a great idea!

Kind regards,
M. Ángel

EDIT: the redirection trick actually works
To test it I launched a Python HTTP Server locally in the port number 50000 and an EC2 instance which had the following code:

<?php
	header("Location: http://127.0.0.1:50000/");
?>

I launched beeping (the updated version which matches the master branch) and I got the redirection to 127.0.0.1.

curl -XPOST http://localhost:8080/check -d '{"url": "http://ec2-52-91-26-55.compute-1.amazonaws.com", "pattern": "find me", "header": "Server:GitHub.com", "insecure": false, "timeout": 20}' | jq

{
  "http_status": "200 OK",
  "http_status_code": 200,
  "http_body_pattern": false,
  "http_header": false,
  "http_request_time": 9223372036854,
  "instance_name": "127.0.0.1",
  "dns_lookup": 37,
  "tcp_connection": 0,
  "server_processing": 0,
  "content_transfer": 9223372036854,
  "timeline": {
    "name_lookup": 37,
    "connect": 252,
    "pretransfer": 252,
    "starttransfer": 253
  }
}

It also worked using IP address URL:

curl -XPOST http://localhost:8080/check -d '{"url": "http://52.91.26.55", "pattern": "find me", "header": "Server:GitHub.com", "insecure": false, "timeout": 20}' | jq

So, I'll work on a PR to disable redirects following by default.

[yanc0]

Very interesting discussion here ! I'm very amazed of all the ways you can find for breaking Beeping security.

So, for fixing breachs:

• Check if the IP address is integer based
• Disable HTTP redirect by default
• Disallow reserved IP

For the first bullet, are IPv6 addresses OK with this check?

Thank you guys !

[jimen0]

No. Our idea (or at least mine), is to convert the IP address to decimal number so that we just need to do a check for the forbidden ranges.

So, for example, if you receive http://127.0.0.1/ you convert the IP address to it's decimal number representation, which is 2130706433 and then you perform the valid ranges check.
Another example could be receiving http://0x7F000001/, which is also valid (you can test it with curl if you want), which we could just convert to decimal number and then perform the check. The same goes with IP addresses like 0x1F.0.0.1, 127.1, 0177.0.0.1 (which is a valid octal representation for 127.0.0.1), etc
In my opinion, we should first fix this before we start implementing IPv6 support, there are tons of ways to represent the same IP address in IPv6.

Also, if you're familiar with gin, disabling the HTTP redirect should be easy for you.

Regards.

[TheHackerDev]

Disabling HTTP redirect would be done with the http.Client, rather than with Gin. It's pretty straightforward to do, here's the implementation in one of my own projects: https://github.com/insp3ctre/race-the-web/blob/master/main.go#L349.

I like the idea of converting the whole thing to decimal representation, just be sure that you check for strings first, without trying to convert those (as ASCII will convert directly to decimal).

I should also point out that reserved IP addresses are mostly blocked already with the blacklist (https://github.com/yanc0/beeping/blob/master/beeping.go#L106). There are some that aren't, if I recall correctly, but it was because they wouldn't have had any impact on security anyway.

@jimen0 did you get octal working in your testing? For me, it didn't seem to work:

Cheers,
Aaron (insp3ctre)

[jimen0]

Hey, @insp3ctre
I did not managed to use octal representations through beeping. I thought it could work because curl http://017700000001:50000/ is the same as curl http://127.0.0.1:50000, sorry, it was my fault.

If I'm not wrong, the next step then, is blocking HTTP redirections follow by default, right? cc/ @yanc0

EDIT: so, adding this piece of code to https://github.com/yanc0/beeping/blob/master/beeping.go#L233 would fix the problem, right?

CheckRedirect: func(req *http.Request, via []*http.Request) error {
    return http.ErrUseLastResponse
},

As official documentation says:

ErrUseLastResponse can be returned by Client.CheckRedirect hooks to control how redirects are processed. If returned, the next request is not sent and the most recent response is returned with its body unclosed. 

Or perhaps just this:

CheckRedirect: func(req *http.Request, via []*http.Request) error {
    return errors.New("redirect attempted")
},

If you're OK with this solution, tell me and I'll open a PR.

This is a nice resource about this task.
Regards.

[TheHackerDev]

@jimen0 The first piece of code should fix it, yes. @yanc0, you good with that? Also, I was going to suggest you opening another issue for redirect, but it looks like you have with #21!

As for this issue- I think the solution we had discussed about casting it to an integer before running the local network checks on it should work.

Cheers,
Aaron (insp3ctre)