Security: Add input validation and memory clearing for sensitive data#22
Merged
Conversation
- SDK-KEYSTORE-001: Clear password buffer from memory after decryption * Convert password string to Uint8Array and fill with zeros in finally block * Ensures sensitive password data is not retained in memory - SDK-INPUT-001: Add comprehensive input validation for transaction inputs * Validate addresses: must be valid Shell addresses (0x + 64 hex chars) or null * Validate amounts: must be non-negative bigints * Validate nonces: must be non-negative integers * Validate gas parameters: must be non-negative integers * Added validateAddress, validateNonNegativeBigInt, validateNonNegativeInteger functions - SDK-RPC-001: Add RPC URL validation * Validate protocol: https/wss for remote, http/ws allowed for localhost only * Reject private IP ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8) * Validate in createShellPublicClient, createShellWsClient, createShellProvider * Added validateRpcUrl function All validations are exported in the public API via validation module. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a reusable validation layer to the Shell SDK and wires it into transaction building, provider/client creation, and keystore decryption to address medium-priority security findings around input validation, RPC URL safety checks, and sensitive data handling.
Changes:
- Introduces
src/validation.tswith validators for addresses, non-negative numbers/bigints, and RPC URLs (protocol + private IPv4 range checks). - Validates transaction inputs in
buildTransaction()before constructing the wire-format request. - Validates RPC URLs in provider/client factory functions and exports validators via the root public API.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/validation.ts | Adds new validation utilities for tx fields and RPC URLs. |
| src/transactions.ts | Calls validators in buildTransaction() to reject invalid tx inputs early. |
| src/provider.ts | Validates RPC URLs before creating HTTP/WS clients/providers. |
| src/keystore.ts | Attempts to clear password material and ensures derived key / secret key are zeroed. |
| src/index.ts | Exposes validation helpers through the root SDK export. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
+74
| const protocol = url.protocol; | ||
| const hostname = url.hostname; | ||
| const isLocal = hostname === "localhost" || hostname === "127.0.0.1" || hostname === "[::1]"; | ||
|
|
Comment on lines
+31
to
+35
| export function validateNonNegativeInteger(value: number, fieldName: string): void { | ||
| if (!Number.isInteger(value) || value < 0) { | ||
| throw new Error(`${fieldName} must be a non-negative integer, got ${value}`); | ||
| } | ||
| } |
Comment on lines
+55
to
+59
| * Rules: | ||
| * - Must be https:// (or http:// for localhost only) | ||
| * - Cannot point to private IP ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8) | ||
| * - WebSocket URLs must start with wss:// (or ws:// for localhost only) | ||
| * |
Comment on lines
+183
to
+190
| // Convert password to Uint8Array for secure erasure | ||
| const passwordBytes = new TextEncoder().encode(password); | ||
|
|
||
| try { | ||
| const chacha = xchacha20poly1305(derivedKey, nonce); | ||
| // Plaintext is sk-only; public key comes from the JSON `public_key` field. | ||
| const secretKey = chacha.decrypt(ciphertext); | ||
| const derivedKeyHex = await argon2id({ | ||
| password, | ||
| salt, | ||
| iterations: ek.kdf_params.t_cost, |
Comment on lines
+18
to
+22
| export function validateNonNegativeBigInt(value: bigint, fieldName: string): void { | ||
| if (typeof value !== "bigint" || value < 0n) { | ||
| throw new Error(`${fieldName} must be a non-negative bigint, got ${value}`); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses three P2 (medium priority) security findings:
SDK-KEYSTORE-001: Keystore password not cleared from memory
Issue: When decrypting a keystore, the password buffer is not zeroed after use, leaving it in memory.
Fix: Password is now converted to Uint8Array and explicitly filled with zeros in a finally block.
Impact: Prevents accidental exposure of sensitive password data in memory dumps.
SDK-INPUT-001: User input validation for addresses and amounts
Issue: SDK may not validate transaction inputs (addresses, amounts, nonces) before signing.
Fix: Added comprehensive input validation layer:
Impact: Prevents invalid transactions from being constructed.
SDK-RPC-001: RPC URL validation
Issue: SDK client doesn't validate RPC URLs before sending requests.
Fix: Added RPC URL validation:
Impact: Prevents accidental connections to unintended or internal network endpoints.
Changes
validation.tsmodule with reusable validatorsbuildTransaction()to validate all inputs before constructingTesting