Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Out of bounds read for keepalive packets #673

Closed
alexbakker opened this issue Feb 26, 2018 · 3 comments
Closed

Out of bounds read for keepalive packets #673

alexbakker opened this issue Feb 26, 2018 · 3 comments

Comments

@alexbakker
Copy link
Contributor

If a keepalive packet is received with a smaller size than expected ((16 bytes for the ip + 2 bytes for the port) * 8), an out of bounds read occurs. This results in peers getting added to the list with seemingly random ip:port combo's.

I suggest fixing this issue by looking at the size of the packet and only parsing as many peers as the size allows, up to 8 peers.

@argakiig
Copy link
Contributor

fixed in #704 feel free to re-open if you do not feel it is resolved

@inkeliz
Copy link

inkeliz commented Jun 1, 2018

I think it still not working, or not as I expect. The node doesn't reply the client if less than 8 peers is sent. It's the correct behavior?

For instance, the message:

5243120F0D020080 000000000000000000000000000000000000

It only includes one single IP, the node didn't reply it. However send "more zeroes" (reaching = 144 bytes) it works:

5243120F0D020080 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 

Have any reason to not reply the first message, when have less than 8 ips?

@cryptocode
Copy link
Contributor

@inkeliz the node always sends 8 entries, initializing to zero if there's not enough peers. A packet with less than 8 is considered invalid. So the behavior you see is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants