diff --git a/eslint-suppressions.json b/eslint-suppressions.json index f82d6d77e..d99483a05 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1037,9 +1037,6 @@ }, "packages/solana-wallet-snap/src/core/validation/structs.ts": { "@typescript-eslint/no-unused-vars": { - "count": 2 - }, - "@typescript-eslint/restrict-template-expressions": { "count": 1 } }, diff --git a/packages/snap-networks-utils/CHANGELOG.md b/packages/snap-networks-utils/CHANGELOG.md index 72df59a78..b35dca48f 100644 --- a/packages/snap-networks-utils/CHANGELOG.md +++ b/packages/snap-networks-utils/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add a `safeMerge` utility for shallowly merging objects. ([#166](https://github.com/MetaMask/internal-snaps/pull/166)) +- Add a `UrlStruct` utility for validating safe HTTP, HTTPS, and WebSocket URLs. ([#174](https://github.com/MetaMask/internal-snaps/pull/174)) ### Changed diff --git a/packages/snap-networks-utils/src/index.ts b/packages/snap-networks-utils/src/index.ts index 9ff931c6d..3718c73db 100644 --- a/packages/snap-networks-utils/src/index.ts +++ b/packages/snap-networks-utils/src/index.ts @@ -9,6 +9,7 @@ export { type RemoteFeatureFlagsProviderMessenger, } from './providers/remote-feature-flags/RemoteFeatureFlagsProvider'; export { safeMerge } from './safeMerge/safeMerge'; +export { UrlStruct } from './urlStruct/urlStruct'; export { Logger, LogLevel } from './logger'; export type { LoggerOptions, diff --git a/packages/snap-networks-utils/src/urlStruct/urlStruct.test.ts b/packages/snap-networks-utils/src/urlStruct/urlStruct.test.ts new file mode 100644 index 000000000..5b7e295e8 --- /dev/null +++ b/packages/snap-networks-utils/src/urlStruct/urlStruct.test.ts @@ -0,0 +1,154 @@ +/* eslint-disable jest/require-to-throw-message */ +import { assert, is } from '@metamask/superstruct'; + +import { UrlStruct } from './urlStruct'; + +describe('UrlStruct', () => { + it('validates valid URLs', () => { + const validUrls = [ + 'http://example.com', + 'https://example.com', + 'https://www.example.com', + 'https://sub.example.com', + 'https://example.com/path', + 'https://example.com/path?query=123', + 'https://example.com/path?query=123&other=456', + 'https://example.com:8080', + 'https://example.com/path-with-hyphens', + 'https://example.com/path_with_underscore', + 'http://localhost:8899', + 'wss://example.com', + ]; + + validUrls.forEach((url) => { + expect(() => assert(url, UrlStruct)).not.toThrow(); + expect(is(url, UrlStruct)).toBe(true); + }); + }); + + it('rejects invalid URLs', () => { + const invalidUrls = [ + '', + 'not-a-url', + 'ftp://example.com', + 'example.com', + 'http:/example.com', + 'http://example', + 'http:///example.com', + 'http:// example.com', + // eslint-disable-next-line no-script-url + 'javascript:alert(1)', + 'data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==', + ]; + + invalidUrls.forEach((url) => { + expect(() => assert(url, UrlStruct)).toThrow(); + expect(is(url, UrlStruct)).toBe(false); + }); + }); + + it('includes the real parser reason when URL format is invalid', () => { + expect(() => assert('http:// example.com', UrlStruct)).toThrow( + 'Invalid URL format: Invalid URL', + ); + }); + + it.each([ + 'https://example%2f.com', + 'https://example.com:abc', + 'https://example.com:342abc', + ])('rejects malformed URL parser input: %s', (url) => { + expect(() => assert(url, UrlStruct)).toThrow( + 'Invalid URL format: Invalid URL', + ); + }); + + it.each(['https://example.com/%', 'https://example.com/%ZZ'])( + 'rejects malformed percent encoding: %s', + (url) => { + expect(() => assert(url, UrlStruct)).toThrow( + 'Invalid URL format: URI malformed', + ); + }, + ); + + it.each(['https://example.com\\path', 'https://user@example.com'])( + 'rejects hostname protocol-pollution input: %s', + (url) => { + expect(() => assert(url, UrlStruct)).toThrow( + 'URL contains protocol pollution attempts', + ); + }, + ); + + it('rejects malicious URLs', () => { + const maliciousUrls = [ + // XSS Attacks + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?
', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + + // Additional XSS Variants + 'https://example.com?', + 'https://example.com?', + 'https://example.com?', + 'https://example.com?
', + + // SQL Injection Attempts + 'https://example.com?id=1%27%20OR%20%271%27=%271', + 'https://example.com?id=1%20UNION%20SELECT%20*%20FROM%20users', + + // Directory Traversal + 'https://example.com/../../../etc/passwd', + 'https://example.com/..%2f..%2f..%2fetc%2fpasswd', + + // Command Injection + 'https://example.com?cmd=|ls', + 'https://example.com?cmd=;cat%20/etc/passwd', + + // Protocol Pollution + 'https://example.com\\@evil.com', + 'https://example.com%2f@evil.com', + 'https://example.com?url=javascript:alert(1)', + + // Unicode/UTF-8 Attacks + 'https://example.com/％2e％2e/％2e％2e/％2e％2e/etc/passwd', + 'https://example.com/⒕⒖⒗', + + // CRLF Injection + 'https://example.com?%0d%0aContent-Length:%200%0d%0a%0d%0aHTTP/1.1%20200%20OK', + + // Open Redirect + 'https://example.com?redirect=//evil.com', + 'https://example.com?url=https://evil.com', + + // HTML Injection without script tags + 'https://example.com?param=test', + 'https://example.com?param=', + + // Data URI schemes + 'data:text/html,', + 'data:application/x-javascript,alert(1)', + + // Null Byte Attacks + 'https://example.com/file.jpg%00.php', + + // Template Injection + // eslint-disable-next-line no-template-curly-in-string + 'https://example.com?${7*7}', + 'https://example.com?#{7*7}', + ]; + maliciousUrls.forEach((url) => { + expect(() => assert(url, UrlStruct)).toThrow(); + expect(is(url, UrlStruct)).toBe(false); + }); + }); +}); diff --git a/packages/snap-networks-utils/src/urlStruct/urlStruct.ts b/packages/snap-networks-utils/src/urlStruct/urlStruct.ts new file mode 100644 index 000000000..ad4e277fe --- /dev/null +++ b/packages/snap-networks-utils/src/urlStruct/urlStruct.ts @@ -0,0 +1,108 @@ +import { refine, string } from '@metamask/superstruct'; + +/** + * Validates that a string is a valid and safe URL. + * + * It rejects: + * - Non-HTTP/HTTPS/WSS protocols + * - Malformed URL format or incorrect protocol format + * - Invalid hostname format (must follow proper domain naming conventions) + * - Protocol pollution attempts (backslashes, @ symbol, %2f@, %5c@) + * - Invalid hostname characters (backslashes, @ symbol, forward slashes, encoded forward slashes) + * - Directory traversal attempts (../, ..%2f, ..%2F) + * + * Dangerous patterns including: + * - HTML tags. + * - JavaScript protocol. + * - Data URI scheme. + * - Template injection (${...}, #{...}). + * - Command injection (|, ;). + * - CRLF injection. + * - URL credential injection. + * - SQL injection attempts. + * - Open redirect parameters. + * - Non-printable characters. + */ +export const UrlStruct = refine(string(), 'safe-url', (value) => { + try { + // Basic URL validation + const url = new URL(value); + + // Protocol check + const supportedProtocols = ['http:', 'https:', 'wss:']; + if (!supportedProtocols.includes(url.protocol)) { + return `URL must use one of the following protocols: ${supportedProtocols.join(', ')}`; + } + + // Validate URL format + if (!value.match(/^(https?|wss):\/\/[^/]+\/?/u)) { + return 'Malformed URL - incorrect protocol format'; + } + + // Validate hostname format. Accepts localhost and ports (needed for tests) + const hostname = url.hostname.toLowerCase(); + if ( + hostname !== 'localhost' && + (!hostname.includes('.') || + !/^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/u.test( + hostname, + )) + ) { + return 'Invalid hostname format'; + } + + // Check for protocol pollution in the entire URL + const decodedValue = decodeURIComponent(value.toLowerCase()); + if ( + value.includes('\\') || + value.includes('@') || + decodedValue.includes('\\') || + decodedValue.includes('@') || + value.toLowerCase().includes('%2f@') || + value.toLowerCase().includes('%5c@') + ) { + return 'URL contains protocol pollution attempts'; + } + + // Check for directory traversal + if ( + value.includes('../') || + value.includes('..%2f') || + value.includes('..%2F') + ) { + return 'Directory traversal attempts are not allowed'; + } + + // Check for dangerous patterns + const dangerousPatterns = [ + /<[^>]*>/u, // HTML tags + /javascript:/u, // JavaScript protocol + /data:/u, // Data URI scheme + /\\[@\\]/u, // Enhanced protocol pollution check + /%2f@/u, // Protocol pollution + /[^\x20-\x7E]/u, // Non-printable characters + /\$\{.*?\}/u, // Template injection + /#\{.*?\}/u, // Template injection + /[|;]/u, // Command injection + /%0[acd]|%0[acd]/u, // CRLF injection + /\/\/\w+@/u, // URL credential injection + // Enhanced SQL injection patterns + /(?:[^a-z]|^)(?:union\s+(?:all\s+)?select|select\s+(?:.*\s+)?from|insert\s+into|update\s+.*\s+set|delete\s+from|drop\s+table|alter\s+table|create\s+table|exec(?:ute)?|union|where\s+[\d\w]\s*=\s*[\d\w]|\bor\b\s*[\d\w]\s*=\s*[\d\w])/iu, + /'.*?(?:OR|UNION|SELECT|FROM|WHERE).*?'/iu, // SQL injection + /%27.*?(?:OR|UNION|SELECT|FROM|WHERE).*?(?:%27|')/iu, // URL-encoded SQL injection + /%20(?:OR|UNION|SELECT|FROM|WHERE)%20/iu, // URL-encoded SQL keywords + /[?&](?:url|redirect|next|return_to|return_url|goto|destination|continue|redirect_uri)=(?:[^&]*\/\/|https?:)/iu, // Open redirect parameters + /[?&](?:url|redirect|next|return_to|return_url|goto|destination|continue|redirect_uri)=%(?:[^&]*\/\/|https?:)/iu, // URL-encoded open redirect parameters + ]; + + for (const patt of dangerousPatterns) { + if (patt.test(decodedValue)) { + return 'URL contains potentially malicious patterns'; + } + } + + return true; + } catch (error) { + return `Invalid URL format: ${(error as Error).message}`; + } +}); diff --git a/packages/solana-wallet-snap/snap.manifest.json b/packages/solana-wallet-snap/snap.manifest.json index 57d2ca99d..67d45a9d0 100644 --- a/packages/solana-wallet-snap/snap.manifest.json +++ b/packages/solana-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "2c2WNzBfLdP/UYJdepbBVpyYrpjzuSob/iobC9JJRLo=", + "shasum": "/PhLDvRV3M/0N7rlAqg7pypEkZAHTfFNP9/TNxi8G80=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/solana-wallet-snap/src/core/clients/nft-api/NftApiClient.ts b/packages/solana-wallet-snap/src/core/clients/nft-api/NftApiClient.ts index 376fc55d4..a53a77a9c 100644 --- a/packages/solana-wallet-snap/src/core/clients/nft-api/NftApiClient.ts +++ b/packages/solana-wallet-snap/src/core/clients/nft-api/NftApiClient.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/naming-convention */ +import { UrlStruct } from '@metamask/snap-networks-utils'; import { assert } from '@metamask/superstruct'; import type { ICache } from '../../caching/ICache'; @@ -8,7 +9,6 @@ import type { Serializable } from '../../serialization/types'; import type { ConfigProvider } from '../../services/config'; import { buildUrl } from '../../utils/buildUrl'; import { trackError } from '../../utils/errors'; -import { UrlStruct } from '../../validation/structs'; import type { Balance, Nft, diff --git a/packages/solana-wallet-snap/src/core/clients/price-api/PriceApiClient.ts b/packages/solana-wallet-snap/src/core/clients/price-api/PriceApiClient.ts index 72dda238a..14c6ef35b 100644 --- a/packages/solana-wallet-snap/src/core/clients/price-api/PriceApiClient.ts +++ b/packages/solana-wallet-snap/src/core/clients/price-api/PriceApiClient.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/naming-convention */ import type { CaipAssetType } from '@metamask/keyring-api'; +import { UrlStruct } from '@metamask/snap-networks-utils'; import type { Logger } from '@metamask/snap-networks-utils'; import { array, assert } from '@metamask/superstruct'; import { CaipAssetTypeStruct } from '@metamask/utils'; @@ -12,7 +13,6 @@ import type { Serializable } from '../../serialization/types'; import type { ConfigProvider } from '../../services/config'; import { buildUrl } from '../../utils/buildUrl'; import logger from '../../utils/logger'; -import { UrlStruct } from '../../validation/structs'; import type { ExchangeRate, FiatTicker, diff --git a/packages/solana-wallet-snap/src/core/clients/token-api-client/TokenApiClient.ts b/packages/solana-wallet-snap/src/core/clients/token-api-client/TokenApiClient.ts index 5c8c71cf5..353de5377 100644 --- a/packages/solana-wallet-snap/src/core/clients/token-api-client/TokenApiClient.ts +++ b/packages/solana-wallet-snap/src/core/clients/token-api-client/TokenApiClient.ts @@ -1,3 +1,4 @@ +import { UrlStruct } from '@metamask/snap-networks-utils'; import type { Logger } from '@metamask/snap-networks-utils'; import type { FungibleAssetMetadata } from '@metamask/snaps-sdk'; import { array, assert } from '@metamask/superstruct'; @@ -9,7 +10,6 @@ import { Network, TokenCaipAssetTypeStruct } from '../../constants/solana'; import type { ConfigProvider } from '../../services/config'; import { buildUrl } from '../../utils/buildUrl'; import logger from '../../utils/logger'; -import { UrlStruct } from '../../validation/structs'; import type { TokenMetadataStruct } from './structs'; import { TokenMetadataResponseStruct } from './structs'; diff --git a/packages/solana-wallet-snap/src/core/clients/token-api-client/structs.ts b/packages/solana-wallet-snap/src/core/clients/token-api-client/structs.ts index f81759066..c9d2538ad 100644 --- a/packages/solana-wallet-snap/src/core/clients/token-api-client/structs.ts +++ b/packages/solana-wallet-snap/src/core/clients/token-api-client/structs.ts @@ -1,3 +1,4 @@ +import { UrlStruct } from '@metamask/snap-networks-utils'; import { array, integer, @@ -7,7 +8,6 @@ import { } from '@metamask/superstruct'; import { TokenCaipAssetTypeFromStringStruct } from '../../constants/solana'; -import { UrlStruct } from '../../validation/structs'; export const TokenMetadataStruct = object({ decimals: integer(), diff --git a/packages/solana-wallet-snap/src/core/services/config/ConfigProvider.ts b/packages/solana-wallet-snap/src/core/services/config/ConfigProvider.ts index 3a25d8f82..5a00610bc 100644 --- a/packages/solana-wallet-snap/src/core/services/config/ConfigProvider.ts +++ b/packages/solana-wallet-snap/src/core/services/config/ConfigProvider.ts @@ -1,5 +1,5 @@ -import { LogLevel } from '@metamask/snap-networks-utils'; /* eslint-disable no-restricted-globals */ +import { UrlStruct, LogLevel } from '@metamask/snap-networks-utils'; import type { Infer } from '@metamask/superstruct'; import { array, @@ -14,7 +14,6 @@ import { uniq } from 'lodash'; import { Network, Networks } from '../../constants/solana'; import { getClientStatus } from '../../utils/interface'; -import { UrlStruct } from '../../validation/structs'; export const SUPPORTED_NETWORKS = [Network.Mainnet, Network.Devnet]; @@ -56,7 +55,7 @@ const EnvStruct = object({ LOCAL_API_BASE_URL: string(), }); -export type Env = Infer; +type Env = Infer; export type NetworkConfig = (typeof Networks)[Network] & { rpcUrls: string[]; diff --git a/packages/solana-wallet-snap/src/core/utils/buildUrl.ts b/packages/solana-wallet-snap/src/core/utils/buildUrl.ts index 1708dcac3..350e5e0eb 100644 --- a/packages/solana-wallet-snap/src/core/utils/buildUrl.ts +++ b/packages/solana-wallet-snap/src/core/utils/buildUrl.ts @@ -1,6 +1,6 @@ +import { UrlStruct } from '@metamask/snap-networks-utils'; import { assert } from '@metamask/superstruct'; -import { UrlStruct } from '../validation/structs'; import { sanitizeControlCharacters, sanitizeUri } from './sanitize'; export type BuildUrlParams = { diff --git a/packages/solana-wallet-snap/src/core/validation/structs.test.ts b/packages/solana-wallet-snap/src/core/validation/structs.test.ts index ad1eed6dd..aa23d1360 100644 --- a/packages/solana-wallet-snap/src/core/validation/structs.test.ts +++ b/packages/solana-wallet-snap/src/core/validation/structs.test.ts @@ -1,7 +1,7 @@ /* eslint-disable jest/require-to-throw-message */ import { assert, is } from '@metamask/superstruct'; -import { Base58Struct, Base64Struct, UrlStruct, UuidStruct } from './structs'; +import { Base58Struct, Base64Struct, UuidStruct } from './structs'; describe('structs', () => { describe('Uuid', () => { @@ -19,122 +19,6 @@ describe('structs', () => { }); }); - describe('UrlStruct', () => { - it('validates valid URLs', () => { - const validUrls = [ - 'http://example.com', - 'https://example.com', - 'https://www.example.com', - 'https://sub.example.com', - 'https://example.com/path', - 'https://example.com/path?query=123', - 'https://example.com/path?query=123&other=456', - 'https://example.com:8080', - 'https://example.com/path-with-hyphens', - 'https://example.com/path_with_underscore', - 'http://localhost:8899', - 'wss://example.com', - ]; - - validUrls.forEach((url) => { - expect(() => assert(url, UrlStruct)).not.toThrow(); - expect(is(url, UrlStruct)).toBe(true); - }); - }); - - it('rejects invalid URLs', () => { - const invalidUrls = [ - '', - 'not-a-url', - 'ftp://example.com', - 'example.com', - 'http:/example.com', - 'http://example', - 'http:///example.com', - 'http:// example.com', - // eslint-disable-next-line no-script-url - 'javascript:alert(1)', - 'data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==', - ]; - - invalidUrls.forEach((url) => { - expect(() => assert(url, UrlStruct)).toThrow(); - expect(is(url, UrlStruct)).toBe(false); - }); - }); - - it('rejects malicious URLs', () => { - const maliciousUrls = [ - // XSS Attacks - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?
', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - - // Additional XSS Variants - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?
', - - // SQL Injection Attempts - 'https://example.com?id=1%27%20OR%20%271%27=%271', - 'https://example.com?id=1%20UNION%20SELECT%20*%20FROM%20users', - - // Directory Traversal - 'https://example.com/../../../etc/passwd', - 'https://example.com/..%2f..%2f..%2fetc%2fpasswd', - - // Command Injection - 'https://example.com?cmd=|ls', - 'https://example.com?cmd=;cat%20/etc/passwd', - - // Protocol Pollution - 'https://example.com\\@evil.com', - 'https://example.com%2f@evil.com', - 'https://example.com?url=javascript:alert(1)', - - // Unicode/UTF-8 Attacks - 'https://example.com/％2e％2e/％2e％2e/％2e％2e/etc/passwd', - 'https://example.com/⒕⒖⒗', - - // CRLF Injection - 'https://example.com?%0d%0aContent-Length:%200%0d%0a%0d%0aHTTP/1.1%20200%20OK', - - // Open Redirect - 'https://example.com?redirect=//evil.com', - 'https://example.com?url=https://evil.com', - - // HTML Injection without script tags - 'https://example.com?param=test', - 'https://example.com?param=', - - // Data URI schemes - 'data:text/html,', - 'data:application/x-javascript,alert(1)', - - // Null Byte Attacks - 'https://example.com/file.jpg%00.php', - - // Template Injection - // eslint-disable-next-line no-template-curly-in-string - 'https://example.com?${7*7}', - 'https://example.com?#{7*7}', - ]; - maliciousUrls.forEach((url) => { - expect(() => assert(url, UrlStruct)).toThrow(); - expect(is(url, UrlStruct)).toBe(false); - }); - }); - }); - describe('Base58Struct', () => { it('validates valid Base58 strings', () => { const validBase58Strings = [ diff --git a/packages/solana-wallet-snap/src/core/validation/structs.ts b/packages/solana-wallet-snap/src/core/validation/structs.ts index 21f7f2ec8..f1eb9accb 100644 --- a/packages/solana-wallet-snap/src/core/validation/structs.ts +++ b/packages/solana-wallet-snap/src/core/validation/structs.ts @@ -11,7 +11,6 @@ import { optional, pattern, record, - refine, string, } from '@metamask/superstruct'; import { address } from '@solana/kit'; @@ -29,129 +28,6 @@ export const PositiveNumberStringStruct = pattern( /^(?!0\d)(\d+(\.\d+)?)$/u, ); -/** - * Validates that a string is a valid and safe URL. - * - * It rejects: - * - Non-HTTP/HTTPS/WSS protocols - * - Malformed URL format or incorrect protocol format - * - Invalid hostname format (must follow proper domain naming conventions) - * - Protocol pollution attempts (backslashes, @ symbol, %2f@, %5c@) - * - Invalid hostname characters (backslashes, @ symbol, forward slashes, encoded forward slashes) - * - Directory traversal attempts (../, ..%2f, ..%2F) - * - * Dangerous patterns including: - * - HTML tags. - * - JavaScript protocol. - * - Data URI scheme. - * - Template injection (${...}, #{...}). - * - Command injection (|, ;). - * - CRLF injection. - * - URL credential injection. - * - SQL injection attempts. - * - Open redirect parameters. - * - Non-printable characters. - */ -export const UrlStruct = refine(string(), 'safe-url', (value) => { - try { - // Basic URL validation - const url = new URL(value); - - // Protocol check - const supportedProtocols = ['http:', 'https:', 'wss:']; - if (!supportedProtocols.includes(url.protocol)) { - return `URL must use one of the following protocols: ${supportedProtocols}`; - } - - // Validate URL format - if (!value.match(/^(https?|wss):\/\/[^/]+\/?/u)) { - return 'Malformed URL - incorrect protocol format'; - } - - // Validate hostname format. Accepts localhost and ports (needed for tests) - const hostname = url.hostname.toLowerCase(); - if ( - hostname !== 'localhost' && - (!hostname.includes('.') || - !/^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/u.test( - hostname, - )) - ) { - return 'Invalid hostname format'; - } - - // Check for protocol pollution in the entire URL - const decodedValue = decodeURIComponent(value.toLowerCase()); - if ( - value.includes('\\') || - value.includes('@') || - decodedValue.includes('\\') || - decodedValue.includes('@') || - value.toLowerCase().includes('%2f@') || - value.toLowerCase().includes('%5c@') - ) { - return 'URL contains protocol pollution attempts'; - } - - // Additional hostname safety check for protocol pollution - const decodedHostname = decodeURIComponent(hostname); - if ( - hostname.includes('\\') || - hostname.includes('@') || - decodedHostname.includes('/') || - hostname.toLowerCase().includes('%2f') - ) { - return 'Invalid hostname characters detected'; - } - - // Check for directory traversal - if ( - value.includes('../') || - value.includes('..%2f') || - value.includes('..%2F') - ) { - return 'Directory traversal attempts are not allowed'; - } - - // Check for dangerous patterns - const dangerousPatterns = [ - /<[^>]*>/u, // HTML tags - /javascript:/u, // JavaScript protocol - /data:/u, // Data URI scheme - /\\[@\\]/u, // Enhanced protocol pollution check - /%2f@/u, // Protocol pollution - /[^\x20-\x7E]/u, // Non-printable characters - /\$\{.*?\}/u, // Template injection - /#\{.*?\}/u, // Template injection - /[|;]/u, // Command injection - /%0[acd]|%0[acd]/u, // CRLF injection - /\/\/\w+@/u, // URL credential injection - // Enhanced SQL injection patterns - /(?:[^a-z]|^)(?:union\s+(?:all\s+)?select|select\s+(?:.*\s+)?from|insert\s+into|update\s+.*\s+set|delete\s+from|drop\s+table|alter\s+table|create\s+table|exec(?:ute)?|union|where\s+[\d\w]\s*=\s*[\d\w]|\bor\b\s*[\d\w]\s*=\s*[\d\w])/iu, - /'.*?(?:OR|UNION|SELECT|FROM|WHERE).*?'/iu, // SQL injection - /%27.*?(?:OR|UNION|SELECT|FROM|WHERE).*?(?:%27|')/iu, // URL-encoded SQL injection - /%20(?:OR|UNION|SELECT|FROM|WHERE)%20/iu, // URL-encoded SQL keywords - /[?&](?:url|redirect|next|return_to|return_url|goto|destination|continue|redirect_uri)=(?:[^&]*\/\/|https?:)/iu, // Open redirect parameters - /[?&](?:url|redirect|next|return_to|return_url|goto|destination|continue|redirect_uri)=%(?:[^&]*\/\/|https?:)/iu, // URL-encoded open redirect parameters - ]; - - for (const patt of dangerousPatterns) { - if (patt.test(decodedValue)) { - return 'URL contains potentially malicious patterns'; - } - } - - // Port validation (if present) - if (url.port && !/^\d+$/u.test(url.port)) { - return 'Invalid port number'; - } - - return true; - } catch (error) { - return 'Invalid URL format'; - } -}); - /** * Keyring validations */ diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index fe36b7959..e13743f95 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "MbqwOXbHFI83/qWOj9zDSXJizgEt5oQ+QnpHq0g/sls=", + "shasum": "EoI9ohkqKuwXmnIAWTRB1z2UGIpDq8PDtYGVtX1GouI=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts b/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts index d513e74ed..7be9b6225 100644 --- a/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts +++ b/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/naming-convention */ import type { CaipAssetType } from '@metamask/keyring-api'; +import { UrlStruct } from '@metamask/snap-networks-utils'; import type { Logger } from '@metamask/snap-networks-utils'; import { array, assert } from '@metamask/superstruct'; import { CaipAssetTypeStruct } from '@metamask/utils'; @@ -13,7 +14,6 @@ import type { ConfigProvider } from '../../services/config'; import { buildUrl } from '../../utils/buildUrl'; import logger from '../../utils/logger'; import type { Serializable } from '../../utils/serialization/types'; -import { UrlStruct } from '../../validation/structs'; import type { FiatExchangeRatesResponse, GetHistoricalPricesParams, diff --git a/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts b/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts index d223f6e42..d0146e2c9 100644 --- a/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts +++ b/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts @@ -1,3 +1,4 @@ +import { UrlStruct } from '@metamask/snap-networks-utils'; import type { Logger } from '@metamask/snap-networks-utils'; import type { FungibleAssetMetadata } from '@metamask/snaps-sdk'; import { array, assert } from '@metamask/superstruct'; @@ -10,7 +11,6 @@ import { TokenCaipAssetTypeStruct } from '../../services/assets/types'; import type { ConfigProvider } from '../../services/config'; import { buildUrl } from '../../utils/buildUrl'; import logger from '../../utils/logger'; -import { UrlStruct } from '../../validation/structs'; import { TokenMetadataResponseStruct } from './structs'; const DEFAULT_DECIMALS = 9; diff --git a/packages/tron-wallet-snap/src/clients/token-api/structs.ts b/packages/tron-wallet-snap/src/clients/token-api/structs.ts index 442d7fc84..fdc231721 100644 --- a/packages/tron-wallet-snap/src/clients/token-api/structs.ts +++ b/packages/tron-wallet-snap/src/clients/token-api/structs.ts @@ -1,3 +1,4 @@ +import { UrlStruct } from '@metamask/snap-networks-utils'; import { array, integer, @@ -7,7 +8,6 @@ import { } from '@metamask/superstruct'; import { TokenCaipAssetTypeStruct } from '../../services/assets/types'; -import { UrlStruct } from '../../validation/structs'; export const TokenMetadataStruct = object({ decimals: integer(), diff --git a/packages/tron-wallet-snap/src/services/config/ConfigProvider.ts b/packages/tron-wallet-snap/src/services/config/ConfigProvider.ts index 4382cccf8..0ca3a4b21 100644 --- a/packages/tron-wallet-snap/src/services/config/ConfigProvider.ts +++ b/packages/tron-wallet-snap/src/services/config/ConfigProvider.ts @@ -1,5 +1,5 @@ -import { LogLevel } from '@metamask/snap-networks-utils'; /* eslint-disable no-restricted-globals */ +import { UrlStruct, LogLevel } from '@metamask/snap-networks-utils'; import type { Infer } from '@metamask/superstruct'; import { array, @@ -12,7 +12,6 @@ import { import { Duration } from '@metamask/utils'; import { Network, Networks } from '../../constants'; -import { UrlStruct } from '../../validation/structs'; const ENVIRONMENT_TO_ACTIVE_NETWORKS = { production: [Network.Mainnet], @@ -49,7 +48,7 @@ const EnvStruct = object({ TRON_HTTP_BASE_URL_SHASTA: UrlStruct, }); -export type Env = Infer; +type Env = Infer; export type NetworkConfig = (typeof Networks)[Network] & { rpcUrls: string[]; diff --git a/packages/tron-wallet-snap/src/utils/buildUrl.ts b/packages/tron-wallet-snap/src/utils/buildUrl.ts index ec1dd5fc6..e86c34eb8 100644 --- a/packages/tron-wallet-snap/src/utils/buildUrl.ts +++ b/packages/tron-wallet-snap/src/utils/buildUrl.ts @@ -1,6 +1,6 @@ +import { UrlStruct } from '@metamask/snap-networks-utils'; import { assert } from '@metamask/superstruct'; -import { UrlStruct } from '../validation/structs'; import { sanitizeControlCharacters, sanitizeUri } from './sanitize'; export type BuildUrlParams = { diff --git a/packages/tron-wallet-snap/src/validation/structs.test.ts b/packages/tron-wallet-snap/src/validation/structs.test.ts index 124fbc187..676f2ddb2 100644 --- a/packages/tron-wallet-snap/src/validation/structs.test.ts +++ b/packages/tron-wallet-snap/src/validation/structs.test.ts @@ -1,7 +1,7 @@ /* eslint-disable jest/require-to-throw-message */ import { assert, is } from '@metamask/superstruct'; -import { Base58Struct, Base64Struct, UrlStruct, UuidStruct } from './structs'; +import { Base58Struct, Base64Struct, UuidStruct } from './structs'; describe('structs', () => { describe('Uuid', () => { @@ -19,128 +19,6 @@ describe('structs', () => { }); }); - describe('UrlStruct', () => { - it('validates valid URLs', () => { - const validUrls = [ - 'http://example.com', - 'https://example.com', - 'https://www.example.com', - 'https://sub.example.com', - 'https://example.com/path', - 'https://example.com/path?query=123', - 'https://example.com/path?query=123&other=456', - 'https://example.com:8080', - 'https://example.com/path-with-hyphens', - 'https://example.com/path_with_underscore', - 'http://localhost:8899', - 'wss://example.com', - ]; - - validUrls.forEach((url) => { - expect(() => assert(url, UrlStruct)).not.toThrow(); - expect(is(url, UrlStruct)).toBe(true); - }); - }); - - it('rejects invalid URLs', () => { - const invalidUrls = [ - '', - 'not-a-url', - 'ftp://example.com', - 'example.com', - 'http:/example.com', - 'http://example', - 'http:///example.com', - 'http:// example.com', - // eslint-disable-next-line no-script-url - 'javascript:alert(1)', - 'data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==', - ]; - - invalidUrls.forEach((url) => { - expect(() => assert(url, UrlStruct)).toThrow(); - expect(is(url, UrlStruct)).toBe(false); - }); - }); - - it('includes the real parser reason when URL format is invalid', () => { - expect(() => assert('http:// example.com', UrlStruct)).toThrow( - 'Invalid URL format: Invalid URL', - ); - }); - - it('rejects malicious URLs', () => { - const maliciousUrls = [ - // XSS Attacks - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?
', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - - // Additional XSS Variants - 'https://example.com?', - 'https://example.com?', - 'https://example.com?', - 'https://example.com?
', - - // SQL Injection Attempts - 'https://example.com?id=1%27%20OR%20%271%27=%271', - 'https://example.com?id=1%20UNION%20SELECT%20*%20FROM%20users', - - // Directory Traversal - 'https://example.com/../../../etc/passwd', - 'https://example.com/..%2f..%2f..%2fetc%2fpasswd', - - // Command Injection - 'https://example.com?cmd=|ls', - 'https://example.com?cmd=;cat%20/etc/passwd', - - // Protocol Pollution - 'https://example.com\\@evil.com', - 'https://example.com%2f@evil.com', - 'https://example.com?url=javascript:alert(1)', - - // Unicode/UTF-8 Attacks - 'https://example.com/％2e％2e/％2e％2e/％2e％2e/etc/passwd', - 'https://example.com/⒕⒖⒗', - - // CRLF Injection - 'https://example.com?%0d%0aContent-Length:%200%0d%0a%0d%0aHTTP/1.1%20200%20OK', - - // Open Redirect - 'https://example.com?redirect=//evil.com', - 'https://example.com?url=https://evil.com', - - // HTML Injection without script tags - 'https://example.com?param=test', - 'https://example.com?param=', - - // Data URI schemes - 'data:text/html,', - 'data:application/x-javascript,alert(1)', - - // Null Byte Attacks - 'https://example.com/file.jpg%00.php', - - // Template Injection - // eslint-disable-next-line no-template-curly-in-string - 'https://example.com?${7*7}', - 'https://example.com?#{7*7}', - ]; - maliciousUrls.forEach((url) => { - expect(() => assert(url, UrlStruct)).toThrow(); - expect(is(url, UrlStruct)).toBe(false); - }); - }); - }); - describe('Base58Struct', () => { it('validates valid Base58 strings', () => { const validBase58Strings = [ diff --git a/packages/tron-wallet-snap/src/validation/structs.ts b/packages/tron-wallet-snap/src/validation/structs.ts index 031c26352..25960cd02 100644 --- a/packages/tron-wallet-snap/src/validation/structs.ts +++ b/packages/tron-wallet-snap/src/validation/structs.ts @@ -17,7 +17,6 @@ import { optional, pattern, record, - refine, string, type, union, @@ -47,129 +46,6 @@ export const PositiveNumberStringStruct = pattern( /^(?!0\d)(\d+(\.\d+)?)$/u, ); -/** - * Validates that a string is a valid and safe URL. - * - * It rejects: - * - Non-HTTP/HTTPS/WSS protocols - * - Malformed URL format or incorrect protocol format - * - Invalid hostname format (must follow proper domain naming conventions) - * - Protocol pollution attempts (backslashes, @ symbol, %2f@, %5c@) - * - Invalid hostname characters (backslashes, @ symbol, forward slashes, encoded forward slashes) - * - Directory traversal attempts (../, ..%2f, ..%2F) - * - * Dangerous patterns including: - * - HTML tags. - * - JavaScript protocol. - * - Data URI scheme. - * - Template injection (${...}, #{...}). - * - Command injection (|, ;). - * - CRLF injection. - * - URL credential injection. - * - SQL injection attempts. - * - Open redirect parameters. - * - Non-printable characters. - */ -export const UrlStruct = refine(string(), 'safe-url', (value) => { - try { - // Basic URL validation - const url = new URL(value); - - // Protocol check - const supportedProtocols = ['http:', 'https:', 'wss:']; - if (!supportedProtocols.includes(url.protocol)) { - return `URL must use one of the following protocols: ${supportedProtocols.join(', ')}`; - } - - // Validate URL format - if (!value.match(/^(https?|wss):\/\/[^/]+\/?/u)) { - return 'Malformed URL - incorrect protocol format'; - } - - // Validate hostname format. Accepts localhost and ports (needed for tests) - const hostname = url.hostname.toLowerCase(); - if ( - hostname !== 'localhost' && - (!hostname.includes('.') || - !/^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/u.test( - hostname, - )) - ) { - return 'Invalid hostname format'; - } - - // Check for protocol pollution in the entire URL - const decodedValue = decodeURIComponent(value.toLowerCase()); - if ( - value.includes('\\') || - value.includes('@') || - decodedValue.includes('\\') || - decodedValue.includes('@') || - value.toLowerCase().includes('%2f@') || - value.toLowerCase().includes('%5c@') - ) { - return 'URL contains protocol pollution attempts'; - } - - // Additional hostname safety check for protocol pollution - const decodedHostname = decodeURIComponent(hostname); - if ( - hostname.includes('\\') || - hostname.includes('@') || - decodedHostname.includes('/') || - hostname.toLowerCase().includes('%2f') - ) { - return 'Invalid hostname characters detected'; - } - - // Check for directory traversal - if ( - value.includes('../') || - value.includes('..%2f') || - value.includes('..%2F') - ) { - return 'Directory traversal attempts are not allowed'; - } - - // Check for dangerous patterns - const dangerousPatterns = [ - /<[^>]*>/u, // HTML tags - /javascript:/u, // JavaScript protocol - /data:/u, // Data URI scheme - /\\[@\\]/u, // Enhanced protocol pollution check - /%2f@/u, // Protocol pollution - /[^\x20-\x7E]/u, // Non-printable characters - /\$\{.*?\}/u, // Template injection - /#\{.*?\}/u, // Template injection - /[|;]/u, // Command injection - /%0[acd]|%0[acd]/u, // CRLF injection - /\/\/\w+@/u, // URL credential injection - // Enhanced SQL injection patterns - /(?:[^a-z]|^)(?:union\s+(?:all\s+)?select|select\s+(?:.*\s+)?from|insert\s+into|update\s+.*\s+set|delete\s+from|drop\s+table|alter\s+table|create\s+table|exec(?:ute)?|union|where\s+[\d\w]\s*=\s*[\d\w]|\bor\b\s*[\d\w]\s*=\s*[\d\w])/iu, - /'.*?(?:OR|UNION|SELECT|FROM|WHERE).*?'/iu, // SQL injection - /%27.*?(?:OR|UNION|SELECT|FROM|WHERE).*?(?:%27|')/iu, // URL-encoded SQL injection - /%20(?:OR|UNION|SELECT|FROM|WHERE)%20/iu, // URL-encoded SQL keywords - /[?&](?:url|redirect|next|return_to|return_url|goto|destination|continue|redirect_uri)=(?:[^&]*\/\/|https?:)/iu, // Open redirect parameters - /[?&](?:url|redirect|next|return_to|return_url|goto|destination|continue|redirect_uri)=%(?:[^&]*\/\/|https?:)/iu, // URL-encoded open redirect parameters - ]; - - for (const patt of dangerousPatterns) { - if (patt.test(decodedValue)) { - return 'URL contains potentially malicious patterns'; - } - } - - // Port validation (if present) - if (url.port && !/^\d+$/u.test(url.port)) { - return 'Invalid port number'; - } - - return true; - } catch (error) { - return `Invalid URL format: ${(error as Error).message}`; - } -}); - /** * Keyring validations */