Insufficient checkings for private IP range #768

New Issue

2015-01-18T00:01:56+01:00

Kagami commented

2015-01-18 00:01:56 +01:00

(Migrated from github.com)

Seems like you forgot to add checking for 172.16.0.0/12 private network range:
https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_receiveDataThread.py#L491
(used in recaddr)

Similar issue: sendaddr uses another routine isHostInPrivateIPRange from helper_generic module which has checking for 172.16/12 but lacks IPv6 support in its turn:
https://github.com/Bitmessage/PyBitmessage/blob/master/src/helper_generic.py#L25

Seems like you forgot to add checking for 172.16.0.0/12 private network range: https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_receiveDataThread.py#L491 (used in `recaddr`) Similar issue: `sendaddr` uses another routine `isHostInPrivateIPRange` from `helper_generic` module which has checking for `172.16/12` but lacks IPv6 support in its turn: https://github.com/Bitmessage/PyBitmessage/blob/master/src/helper_generic.py#L25

Kagami commented

2015-02-04 21:40:54 +01:00

(Migrated from github.com)

Btw, in the logs:

$ grep -P "['\s]172\.(1[6-9]|2[0-9]|3[0-1])\." bm.log
added new node Peer(host='172.17.182.79', port=8444) to knownNodes in stream 1
Could NOT connect to Peer(host='172.17.182.79', port=8444) during outgoing attempt. [Errno 113] No route to host
Could NOT connect to Peer(host='172.17.182.79', port=8444) during outgoing attempt. [Errno 113] No route to host
Could NOT connect to Peer(host='172.17.182.79', port=8444) during outgoing attempt. [Errno 113] No route to host
<singleListener(Thread-70, started daemon 140031427602176)> connected to 172.17.42.1 during INCOMING request.
Timeout occurred waiting for data from Peer(host='172.17.42.1', port=57744) . Closing receiveData thread. (ID: 140031503757648)
sendDataThread (associated with Peer(host='172.17.42.1', port=57744) ) ID: 140036211242576 shutting down now.

(172.17.42.1 is my attempt.)
Not very serious but can lead to some sort of SSRF attacks.

Btw, in the logs: ``` $ grep -P "['\s]172\.(1[6-9]|2[0-9]|3[0-1])\." bm.log added new node Peer(host='172.17.182.79', port=8444) to knownNodes in stream 1 Could NOT connect to Peer(host='172.17.182.79', port=8444) during outgoing attempt. [Errno 113] No route to host Could NOT connect to Peer(host='172.17.182.79', port=8444) during outgoing attempt. [Errno 113] No route to host Could NOT connect to Peer(host='172.17.182.79', port=8444) during outgoing attempt. [Errno 113] No route to host <singleListener(Thread-70, started daemon 140031427602176)> connected to 172.17.42.1 during INCOMING request. Timeout occurred waiting for data from Peer(host='172.17.42.1', port=57744) . Closing receiveData thread. (ID: 140031503757648) sendDataThread (associated with Peer(host='172.17.42.1', port=57744) ) ID: 140036211242576 shutting down now. ``` (`172.17.42.1` is my attempt.) Not very serious but can lead to some sort of SSRF attacks.

Kagami commented

2015-02-17 14:23:21 +01:00

(Migrated from github.com)

As I mentioned in DevTalk, there is also another problem with addr messages: PyBitmessage doesn't check private ranges when it advertises connected peer's address:
https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_receiveDataThread.py#L275
https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_sendDataThread.py#L121

As I mentioned in DevTalk, there is also another problem with `addr` messages: PyBitmessage doesn't check private ranges when it advertises connected peer's address: https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_receiveDataThread.py#L275 https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_sendDataThread.py#L121

PeterSurda commented

2015-11-12 17:34:50 +01:00

(Migrated from github.com)

If I understand correctly, once I do what you request, among other things, it will become less likely that PyBitmessage will attempt to connect to hosts within a private range, even if they actually are in the same local network. Is this intentional?

Also, isn't it enough to change the recaddr function (the first one of the four changes you requested)? Then only those IPs you already have stored locally will be tried to connect to, they will soon time out and it won't accept any more IPs from these ranges. In the worst case, people who don't upgrade will be statistically more affected than if the changes 2-4 were implemented too.

Or do I misunderstand something?

If I understand correctly, once I do what you request, among other things, it will become less likely that PyBitmessage will attempt to connect to hosts within a private range, **even if they actually are in the same local network**. Is this intentional? Also, isn't it enough to change the recaddr function (the first one of the four changes you requested)? Then only those IPs you already have stored locally will be tried to connect to, they will soon time out and it won't accept any more IPs from these ranges. In the worst case, people who don't upgrade will be statistically more affected than if the changes 2-4 were implemented too. Or do I misunderstand something?

Kagami commented

2015-11-12 17:50:29 +01:00

(Migrated from github.com)

it will become less likely that PyBitmessage will attempt to connect to hosts within a private range

That's how it works now, just some ranges are missed. I think trustedpeer option is intended for connections to local peers.

even if they actually are in the same local network

Local network for one peer very probably is not the same for the other. Note also SSRF type of attacks.

Also, isn't it enough to change the recaddr function

I think it'll still try to advertise e.g. local trusted peers which doesn't make sense.

> it will become less likely that PyBitmessage will attempt to connect to hosts within a private range That's how it works now, just some ranges are missed. I think `trustedpeer` option is intended for connections to local peers. > even if they actually are in the same local network Local network for one peer very probably is not the same for the other. Note also SSRF type of attacks. > Also, isn't it enough to change the recaddr function I think it'll still try to advertise e.g. local trusted peers which doesn't make sense.

PeterSurda commented

2016-01-26 12:05:26 +01:00

(Migrated from github.com)

Review @Kagami if you'd like, I believe this is fixed now.

This repo is archived. You cannot comment on issues.