use of K6_BLACKLIST_IPS causes nil pointer dereference

[krashanoff]

Environment

k6 v0.26.2 (dev build, go1.14.4, linux/amd64)

• k6 version: v0.26.2
• OS and version: Linux 5.7.0-3
• Docker version and image, if applicable: N/A

Expected Behavior

Use of the K6_BLACKLIST_IPS envconfig variable should blacklist the provided IPs.

Actual Behavior

A nil pointer is dereferenced in lib.(*IPNet).UnmarshalText. The program crashes.

Steps to Reproduce the Problem

1. Run k6 with an arbitrary script and arbitrary CIDR string passed to the K6_BLACKLIST_IPS environment variable.
   • K6_BLACKLIST_IPS='some.cidr.str.i.ng/0' k6 run path/to/script.js
   • Alternatively, set the variable in advance with export K6_BLACKLIST_IPS='some.cidr.str.i.ng/0', then run k6 run path/to/script.js.
2. A nil pointer is dereferenced in the aforementioned function call.

Extra Information

Here is a full dump of the test I used to reproduce:

~/.../repos/k6 >>> K6_BLACKLIST_IPS='192.111.0.2/8' k6 run samples/test.js

          /\      |‾‾|  /‾‾/  /‾/   
     /\  /  \     |  |_/  /  / /    
    /  \/    \    |      |  /  ‾‾\  
   /          \   |  |‾\  \ | (_) | 
  / __________ \  |__|  \__\ \___/ .io

panic: runtime error: invalid memory address or nil pointer dereferenceptions
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x98624d]

goroutine 1 [running]:
github.com/loadimpact/k6/lib.(*IPNet).UnmarshalText(0x0, 0xc000c213b0, 0xd, 0x10, 0xd, 0x10)
        /home/leo/go/src/github.com/loadimpact/k6/lib/options.go:173 +0x7d
github.com/loadimpact/k6/vendor/github.com/kelseyhightower/envconfig.processField(0xc000038651, 0xd, 0xd47fa0, 0xc0000c22b0, 0x196, 0xc0000c22b0, 0x196)
        /home/leo/go/src/github.com/loadimpact/k6/vendor/github.com/kelseyhightower/envconfig/envconfig.go:251 +0x1262
github.com/loadimpact/k6/vendor/github.com/kelseyhightower/envconfig.processField(0xc000038651, 0xd, 0xce1ac0, 0xc0021e5d38, 0x197, 0x0, 0x0)
        /home/leo/go/src/github.com/loadimpact/k6/vendor/github.com/kelseyhightower/envconfig/envconfig.go:312 +0xf83
github.com/loadimpact/k6/vendor/github.com/kelseyhightower/envconfig.Process(0x0, 0x0, 0xd5a9c0, 0xc0021e5c00, 0x49ad1d, 0x107f360)
        /home/leo/go/src/github.com/loadimpact/k6/vendor/github.com/kelseyhightower/envconfig/envconfig.go:215 +0x47a
github.com/loadimpact/k6/cmd.readEnvConfig(0x109c500, 0x19686c0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/leo/go/src/github.com/loadimpact/k6/cmd/config.go:188 +0x69
github.com/loadimpact/k6/cmd.getConsolidatedConfig(0x109c500, 0x19686c0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/leo/go/src/github.com/loadimpact/k6/cmd/config.go:311 +0x90f
github.com/loadimpact/k6/cmd.glob..func11(0x192fba0, 0xc000272030, 0x1, 0x1, 0x0, 0x0)
        /home/leo/go/src/github.com/loadimpact/k6/cmd/run.go:139 +0x817
github.com/loadimpact/k6/vendor/github.com/spf13/cobra.(*Command).execute(0x192fba0, 0xc000272000, 0x1, 0x1, 0x192fba0, 0xc000272000)
        /home/leo/go/src/github.com/loadimpact/k6/vendor/github.com/spf13/cobra/command.go:762 +0x453
github.com/loadimpact/k6/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x192f220, 0xc00025ec00, 0xc0000be980, 0xe63b4d)
        /home/leo/go/src/github.com/loadimpact/k6/vendor/github.com/spf13/cobra/command.go:852 +0x2ea
github.com/loadimpact/k6/vendor/github.com/spf13/cobra.(*Command).Execute(...)
        /home/leo/go/src/github.com/loadimpact/k6/vendor/github.com/spf13/cobra/command.go:800
github.com/loadimpact/k6/cmd.Execute()
        /home/leo/go/src/github.com/loadimpact/k6/cmd/root.go:89 +0x3f
main.main()
        /home/leo/go/src/github.com/loadimpact/k6/main.go:28 +0x20

[krashanoff]

I actually found this issue while I was working on adding the --block-hostname option (#972). Is it alright if I take this on if the issue is found to be reproducible? Should be a (hopefully) small fix.

[na--]

Thanks for reporting this, though I think this is a duplicate of #1257?

[krashanoff]

Yes it is! Sorry about that, my mistake.

[na--]

No worries 😄 Regarding:

Is it alright if I take this on if the issue is found to be reproducible?

Absolutely no objections on my part, just consider that you might be biting off more than you realize. If #1257 (comment) is accurate, and I think it is, then solving this issue might require replacing the envconfig dependency we use. We want to do it for other reasons besides this one, IIRC this is the third major issue we've had with it, other 2 issues are described in #671... So, it seems to just be a dependency constantly causing issues that we can't really work around 😞

[krashanoff]

just consider that you might be biting off more than you realize.

100% yes.

Shortly after sending that, I looked into the issues that have stemmed from this dep and there's ...a lot. I think for the time being these bugs will be contingent on kelseyhightower/envconfig/pull/158, but I also noticed there's comments looking to refactor the whole configuration process. Definitely a big undertaking that I don't think I'll be diving into quite yet.

In any case, thanks!

[na--]

Yeah, we have a lot of things to refactor in the k6 configuration... 😞 #883 is the epic issue we collect and connect individual issues, if you want an overview of the issues...

[krashanoff]

I'll take a peek at it when I get a minute tomorrow, thanks for being so responsive. Take it easy 👍