client-native icon indicating copy to clipboard operation
client-native copied to clipboard

nil pointer dereference crash when using `PrepareACL` when `MasterSocket` is used

Open shadyabhi opened this issue 1 year ago • 1 comments

In this location, we use err.Error() after a potential situation of err == nil.

https://github.com/haproxytech/client-native/blob/bee8caf96cec8431a372773d15ad657086fb8363/runtime/acls.go#L177

	response, err := s.ExecuteWithResponse(cmd)
	if err != nil {
		return "", fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)
	}
	parts := strings.Split(response, ":")
	if len(parts) < 3 {
		return "", fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)
	}

If there's no error in s.ExecuteWithResponse(cmd), err is nil and then later if len(parts) < 3 , then there's a crash scenario because we're calling function .Error() on a nil err.

I'm observing that in my tests.

Environment:

HAproxy version: 2.8.0-01b97d-15, this behavior is confirmed in 2.9-dev9-f1f8e2-108 (master HEAD, as of today) as well. clientnative: v4.1.3 This is an older library, but I see a similar scenario possibility in HEAD, as shared above. client-native socket is configured to work with MasterSocket.

After some debugging, the response of "prepare acl" command is missing "first part, note the missing : in the beginning".

image

If I use runtimeoptions.Socket instead of runtime.MasterSocket, the response is different, which doesn't trigger this condition.

image

It seems that set severity-output; is ignored in MasterSocket scenario.

➤ echo 'set severity-output number; prepare acl #18' | socat stdio /export/content/lid/apps/traffic-gateway/i001/haproxy.sock

[6]: New version created: 29

➤ echo '@1 set severity-output number;@1 prepare acl #18' | socat stdio /export/content/lid/apps/traffic-gateway/i001/master.sock

New version created: 30

Stacktrace when using runtimeoptions.MasterSocket:-

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xddce2f]

goroutine 21 [running]:
testing.tRunner.func1.2({0xf14300, 0x157afa0})
        /home/arastogi/.gradle/language/golang/1.19/go/src/testing/testing.go:1396 +0x372
testing.tRunner.func1()
        /home/arastogi/.gradle/language/golang/1.19/go/src/testing/testing.go:1399 +0x5f0
panic({0xf14300, 0x157afa0})
        /home/arastogi/.gradle/language/golang/1.19/go/src/runtime/panic.go:890 +0x262
github.com/haproxytech/client-native/v4/runtime.(*SingleRuntime).PrepareACL(0x0?, {0xffcff0, 0x5d})
        /basedir/src/github.com/haproxytech/client-native/v4/runtime/acls.go:177 +0x1cf
github.com/haproxytech/client-native/v4/runtime.(*client).AddACLAtomic(0xc0003fe8c0, {0xffcff0, 0x5d}, {0xc000400790, 0x2, 0x0?})
        /basedir/src/github.com/haproxytech/client-native/v4/runtime/runtime_client.go:1125 +0x493

shadyabhi avatar Nov 17 '23 02:11 shadyabhi

Is it fair to say that the action item for this one is to simply wait for the fix on HAproxy side?

shadyabhi avatar Nov 27 '23 23:11 shadyabhi