dominikh
Flag using %s:%d to construct network addresses
Flag uses of fmt.Sprintf("%s:%d", ...) for constructing network addresses that are to be passed to net.Dial, for IPv6 reasons.
https://github.com/golang/go/issues/28308, which is where we took the idea from, proposes an implementation based on argument names to Sprintf. I think that's too fragile, and that we can do better. We can detect net.Dial (et al) wrapper functions, and we can follow the value returned by fmt.Sprintf as it flows to such a function (i.e., data flow analysis)
I think there's a low hanging fruit here whereby we can just inspect the SSA form and check if one of the arguments to net.Dial is an *ssa.Call to fmt.Sprintf that has certain properties (number of arguments, format string value).
This looks like it'd cover a lot of simple cases and not have too many false positives. WDYT?
There is of course the approach of inspecting direct calls to net.Dial, which may catch some bugs. However, it will miss a lot of bugs. Most people do not use net.Dial directly, but rather an application layer protocol, e.g. net/http. For that, we need to detect net.Dial wrappers, i.e. functions that call net.Dial. Not only that, we have to track the flow of arguments from the wrapper to the net.Dial call, to make sure that our fmt.Sprintf'ed value is in fact used as an argument to net.Dial, and not for something unrelated.
Additionally, we need to be sure that the arguments to fmt.Sprintf are problematic, which involves more data flow analysis. net.Dial(fmt.Sprintf("%s:%d", x, y)) may well be correct if x is formatted appropriately for IPv6 addresses.
Another pattern to flag:
strings.Join([]string{host, port}, ":")(Although it should be noted, that the slice may or may not be a literal.)
Also, while we're at it, using strings.Split instead of
net.SplitHostPort should be flagged as well.
See https://go.dev/cl/554495 for a quick prototype implementation and preliminary results.
