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

Prevent a nil pointer dereference segfault #120

Merged

Conversation

boydjeff
Copy link
Contributor

A nil pointer dereference segfault may occur when [ipv4, ipv6].NewPacketConn() is called with an argument that is nil. The four arguments to the four calls to NewPacketConn() may be nil if the functions that set them, inside the newClient() function, return a non-nil error.

In newClient() there are two calls to net.ListenUDP that set uconn4 and uconn6, and two calls to net.ListenMulticastUDP that set mconn4 and mconn6. There are network scenarios where multicast may be allowed for IPv4 addresses, but not IPv6 addresses, for example. In this case the check in newClient() that returns an error, (if mconn4 == nil && mconn6 == nil), is not hit because only mconn6 is nil. Then, ipv6MulticastConn is set to nil in the returned client object. This nil value for ipv6MulticastConn is then used as an argument to ipv6.NewPacketConn in setInterface() and a nil pointer dereference segfault occurs.

This PR prevents the segfault by checking that each protocol (IPv4 and IPv6) has both unicast and multicast connections. If either unicast or multicast is not present for a protocol, then both are disabled and the use_ipv[4, 6] flag is set to false.

A nil pointer dereference segfault may occur when [ipv4, ipv6].NewPacketConn() is called with an argument that is nil. The four arguments to the four calls to NewPacketConn() may be nil if the functions that set them, inside the newClient() function, return a non-nil error.

In newClient() there are two calls to net.ListenUDP that set uconn4 and uconn6, and two calls to net.ListenMulticastUDP that set mconn4 and mconn6. There are network scenarios where multicast may be allowed for IPv4 addresses, but not IPv6 addresses, for example. In this case the check in newClient() that returns an error, (if mconn4 == nil && mconn6 == nil), is not hit because only mconn6 is nil. Then, ipv6MulticastConn is set to nil in the returned client object. This nil value for ipv6MulticastConn is then used as an argument to ipv6.NewPacketConn in setInterface() and a nil pointer dereference segfault occurs.

This PR prevents the segfault by checking that each protocol (IPv4 and IPv6) has both unicast and multicast connections. If either unicast or multicast is not present for a protocol, then both are disabled and the `use_ipv[4, 6]` flag is set to false.
@boydjeff boydjeff requested a review from a team as a code owner April 19, 2024 23:56
@boydjeff
Copy link
Contributor Author

This PR is a result from feedback in Issue #119.

Copy link
Contributor

@DanStough DanStough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much appreciate the new PR, @boydjeff. A couple minor changes suggested based on a recent PR to merge.

client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
Copy link
Contributor

@DanStough DanStough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@DanStough DanStough merged commit e208a10 into hashicorp:main May 3, 2024
2 checks passed
@boydjeff
Copy link
Contributor Author

boydjeff commented May 3, 2024

LGTM, thanks!

Thanks @DanStough, I really appreciate you working with me on this!

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

Successfully merging this pull request may close these issues.

2 participants