Skip to content

fix: add nil checks for KeyPair in SaveAccountWorker - #539

Open
Kubudak90 wants to merge 1 commit into
coinbase:masterfrom
Kubudak90:fix-saveaccount-nil-check
Open

fix: add nil checks for KeyPair in SaveAccountWorker#539
Kubudak90 wants to merge 1 commit into
coinbase:masterfrom
Kubudak90:fix-saveaccount-nil-check

Conversation

@Kubudak90

Copy link
Copy Markdown

Description

The SaveAccountWorker function validates AccountIdentifier using asserter.AccountIdentifier() but does not validate KeyPair before passing it to StoreKey. This could cause nil pointer dereference panics if KeyPair is nil or has nil/empty fields.

Changes

Add validation for:

  • KeyPair is not nil
  • KeyPair.PublicKey is not nil
  • KeyPair.PrivateKey is not empty

Bug Details

The KeyPair struct contains:

  • PublicKey *types.PublicKey (pointer, can be nil)
  • PrivateKey []byte (can be nil/empty)

If these fields are nil/empty, StoreKey could panic or store invalid data.

Testing

This fix follows the same validation pattern used elsewhere in the codebase (e.g., asserter.AccountIdentifier, asserter.PublicKey).

Fixes potential nil pointer dereference in constructor worker.

The SaveAccountWorker function validates AccountIdentifier but does not
validate KeyPair before passing it to StoreKey. This could cause nil
pointer dereference panics if KeyPair is nil or has nil/empty fields.

Add validation for:
- KeyPair is not nil
- KeyPair.PublicKey is not nil
- KeyPair.PrivateKey is not empty

This follows the same validation pattern used elsewhere in the codebase.
@cb-heimdall

Copy link
Copy Markdown

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants