Ping.Unix falsely returns successful pings

[danzel]

Our app pings a range of IPs, and occasionally it 'receives' a ping reply from a host that doesn't exist on the network.

//Roughly like this
for (var ipDecimal = 1; ipDecimal < 255; ipDecimal++)
{
	var ping = new Ping();
	pingTasks.Add(ping.SendPingAsync("192.168.0." + ipDecimal, 1000));
}
Task.WaitAll(pingTasks.ToArray(), cancellationToken);
foreach (var res in pingTasks.Select(t => t.Result))
{
	if (res.Status == IPStatus.Success)
		Console.WriteLine("Response from " + res.Address);
}

(App is running with RawSockets permissions)

Checking the Ping code, it uses a random Identifier for the ping packet generated from a [ThreadStatic] Random
https://github.com/dotnet/corefx/blob/master/src/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs#L21-L22
https://github.com/dotnet/corefx/blob/master/src/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs#L71-L73

Then when an EchoRequest comes in, it compares that Identifier to check if it matches what was sent out (it doesn't check the IP address as far as I can tell)
https://github.com/dotnet/corefx/blob/master/src/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs#L103

What I guess is happening is that we randomly generate the same Identifier for a host that exists and a host that doesn't, so when we get the reply from the host that does exist, it gets incorrect counted as a reply for the host that doesn't exist.
I haven't done a packet capture to verify this is the cause, it takes days to reproduce, but can do so if necessary.

Could we compare the IP Address the reply is coming from, or use a shared counter for the Identifier or something to prevent reusing the same Identifier quickly?

The Ping code is pretty extensive, so I may have misinterpreted something.
Thanks!

[karelz]

If comparing IP address is going to be correct (no magic forwarding, etc.), then I'd be ok with that.
@wfurt @tmds any thoughts?

[wfurt]

that would make sense. It would be nice to get simple repro app to confirm that theory.
Also note that one could ping broadcast address (or IPv6 any node).
With that the address will not match but that could possibly be handled as special case.

[danzel]

Here is a test app:

using System;
using System.Net.NetworkInformation;
 
namespace PingTest
{
        class Program
        {
                static void Main(string[] args)
                {
                        Console.WriteLine("Hello World!");
                        const string ExistingIp = "192.168.0.10";
                        const string NonExistingIp = "192.168.0.11";
 
                        PingIt(NonExistingIp, 1000* 1000, true);
 
                        for (var i = 0; i < 60 * 1000; i++)
                        {
                                PingIt(ExistingIp, 100, false);
                        }
                }
 
                static async void PingIt(string ip, int timeout, bool printOutput)
                {
                        var ping = new Ping();
                        var res = await ping.SendPingAsync(ip, timeout);
 
                        if (printOutput)
                                Console.WriteLine(res.Status + " " + ip);
                }
        }
}

Set NonExistingIp to a non-existing IP on your local network.
And set ExistingIp to an existing IP address on your network (That doesn't mind you sending them heaps of pings)
And remember to run it as sudo.

In a packet capture you see an arp request for the NonExistingIp (which never gets a response, so we never actually send a ping).
Then after a moment you'll see Success 192.168.0.11.

[wfurt]

I was able to reproduce and get successful ping to non-existing address. The UnixPing works as expected when running without root privilege.