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

x/auth/types: ModuleAccount.Validate() should check that its .BaseAccount is non-nil lest experience a crash #16552

Closed
odeke-em opened this issue Jun 14, 2023 · 0 comments · Fixed by #16554
Labels

Comments

@odeke-em
Copy link
Collaborator

Summary of Bug

I found this panic but fuzzing cosmos/gaia/app whereby
This code in

if ma.Address != sdk.AccAddress(crypto.AddressHash([]byte(ma.Name))).String() {
presumes that .Address is always set having come from a composed ModuleAccount.BaseAccount, but that's a blind assumption which leads to a panic just by this code

func TestModuleAccountValidateNilBaseAccount(t *testing.T) {
        ma := &types.ModuleAccount{Name: "foo"}
        _ = ma.Validate()
}

which results in this crash

$ go test -run=NilBaseAccount
--- FAIL: TestModuleAccountValidateNilBaseAccount (0.00s)
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=0x8 pc=0x100f4352d]

goroutine 417 [running]:
testing.tRunner.func1.2({0x1016e75a0, 0x102d7e660})
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1548 +0x397
panic({0x1016e75a0?, 0x102d7e660?})
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/panic.go:914 +0x21f
github.com/cosmos/cosmos-sdk/x/auth/types.ModuleAccount.Validate({0x0, {0x1019ae75b, 0x3}, {0x0, 0x0, 0x0}})
	/Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/x/auth/types/account.go:221 +0xed
github.com/cosmos/cosmos-sdk/x/auth/types_test.TestModuleAccountValidateNilBaseAccount(0x0?)
	/Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/x/auth/types/genesis_test.go:141 +0x4e
testing.tRunner(0xc000568000, 0x101b497b0)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1595 +0xff
created by testing.(*T).Run in goroutine 1
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1648 +0x3ad
exit status 2
FAIL	github.com/cosmos/cosmos-sdk/x/auth/types	0.614s

Version

The latest at 7c54a1c

Remedy

Please check that ma.BaseAccount is non-nil

@@ -224,10 +217,7 @@
 	if strings.TrimSpace(ma.Name) == "" {
 		return errors.New("module account name cannot be blank")
 	}
-        if ma.BaseAccount == nil {
-                return errors.New("uninitialized ModuleAccount: BBaseAccount is nil")
-        }
-        
+

/cc @elias-orijtech

@odeke-em odeke-em added the T:Bug label Jun 14, 2023
odeke-em added a commit that referenced this issue Jun 14, 2023
…ount.Validate

This change ensures that ModuleAccount.Validate flags nil .BaseAccount
to avoid a nil pointer dereference. This bug was found by fuzzing
cosmos/gaia.

Fixes #16552
odeke-em added a commit that referenced this issue Jun 14, 2023
…ount.Validate

This change ensures that ModuleAccount.Validate flags nil .BaseAccount
to avoid a nil pointer dereference. This bug was found by fuzzing
cosmos/gaia.

Fixes #16552
odeke-em added a commit that referenced this issue Jun 14, 2023
…ount.Validate

This change ensures that ModuleAccount.Validate flags nil .BaseAccount
to avoid a nil pointer dereference. This bug was found by fuzzing
cosmos/gaia.

Fixes #16552
odeke-em added a commit that referenced this issue Jun 14, 2023
…ount.Validate

This change ensures that ModuleAccount.Validate flags nil .BaseAccount
to avoid a nil pointer dereference. This bug was found by fuzzing
cosmos/gaia.

Fixes #16552
odeke-em added a commit that referenced this issue Jun 14, 2023
…ount.Validate

This change ensures that ModuleAccount.Validate flags nil .BaseAccount
to avoid a nil pointer dereference. This bug was found by fuzzing
cosmos/gaia.

Fixes #16552
odeke-em added a commit that referenced this issue Jun 15, 2023
…ount.Validate

This change ensures that ModuleAccount.Validate flags nil .BaseAccount
to avoid a nil pointer dereference. This bug was found by fuzzing
cosmos/gaia.

Fixes #16552
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant