-
Couldn't load subscription status.
- Fork 525
fix: prevent .ipfs. duplication in subdomain gateway URLs #2449
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
base: main
Are you sure you want to change the base?
Changes from all commits
4101e4f
8ab8a78
61d9d11
327367b
973c80b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,109 @@ export const checkValidHttpUrl = (value) => { | |
| return url.protocol === 'http:' || url.protocol === 'https:' | ||
| } | ||
|
|
||
| /** | ||
| * Security validation for subdomain gateway URLs | ||
| * @param {string} gatewayUrl - The gateway URL to validate | ||
| * @returns {{isValid: boolean, warnings: string[], errors: string[], securityScore: number}} - Validation result with security warnings | ||
| */ | ||
| export const validateSubdomainGatewaySecurity = (gatewayUrl) => { | ||
| const warnings = [] | ||
| const errors = [] | ||
|
|
||
| try { | ||
| const url = new URL(gatewayUrl) | ||
|
|
||
| // Check for HTTPS | ||
| if (url.protocol !== 'https:') { | ||
| warnings.push('Gateway should use HTTPS for security') | ||
| } | ||
|
|
||
| // Check for localhost/private IPs in production | ||
| const hostname = url.hostname.toLowerCase() | ||
| const isLocalhost = hostname === 'localhost' || | ||
| hostname === '127.0.0.1' || | ||
| hostname.startsWith('192.168.') || | ||
| hostname.startsWith('10.') || | ||
| hostname.startsWith('172.') | ||
|
|
||
| if (isLocalhost) { | ||
| warnings.push('Using localhost/private IP gateway may not be accessible to other users') | ||
| } | ||
|
|
||
| // Check for potentially risky subdomain patterns | ||
| if (hostname.includes('ipfs.') && !hostname.startsWith('ipfs.')) { | ||
| warnings.push('Subdomain gateway with "ipfs." in the middle may cause confusion') | ||
| } | ||
|
|
||
| // Check for suspicious domains | ||
| const suspiciousPatterns = [ | ||
| /\.tk$/, /\.ml$/, /\.ga$/, /\.cf$/, // Free domains often used for malicious purposes | ||
| /\.onion$/, // Tor hidden services | ||
| /localhost\./, /127\.0\.0\.1\./, /0\.0\.0\.0\./ // Localhost subdomains | ||
| ] | ||
|
|
||
| for (const pattern of suspiciousPatterns) { | ||
| if (pattern.test(hostname)) { | ||
| warnings.push('Gateway domain may be unreliable or potentially malicious') | ||
| break | ||
| } | ||
| } | ||
|
Comment on lines
+88
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a place to make judgement about domain being safe or not. We should only test if domain is a valid subdomain gateway and if it correctly implements URL convention and CORS headers. Please remove this. |
||
|
|
||
| // Check for very short domains (potential typosquatting) | ||
| const domainParts = hostname.split('.') | ||
| if (domainParts.length >= 2) { | ||
| const tld = domainParts[domainParts.length - 1] | ||
| const domain = domainParts[domainParts.length - 2] | ||
| if (domain.length <= 2 && tld.length <= 2) { | ||
| warnings.push('Very short domain name may be a typo or malicious') | ||
| } | ||
| } | ||
|
Comment on lines
+102
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary complexity |
||
| } catch (error) { | ||
| errors.push('Invalid URL format') | ||
| } | ||
|
|
||
| return { | ||
| isValid: errors.length === 0, | ||
| warnings, | ||
| errors, | ||
| securityScore: calculateSecurityScore(gatewayUrl, warnings, errors) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Calculate a security score for the gateway URL | ||
| * @param {string} gatewayUrl - The gateway URL | ||
| * @param {string[]} warnings - Array of warnings | ||
| * @param {string[]} errors - Array of errors | ||
| * @returns {number} - Security score from 0-100 | ||
| */ | ||
| const calculateSecurityScore = (gatewayUrl, warnings, errors) => { | ||
| if (errors.length > 0) return 0 | ||
|
|
||
| let score = 100 | ||
|
|
||
| // Deduct points for warnings | ||
| score -= warnings.length * 10 | ||
|
|
||
| // Bonus points for HTTPS | ||
| if (gatewayUrl.startsWith('https://')) { | ||
| score += 5 | ||
| } | ||
|
|
||
| // Bonus points for well-known domains | ||
| const wellKnownDomains = [ | ||
| 'dweb.link', 'ipfs.io', 'gateway.pinata.cloud', | ||
| 'cloudflare-ipfs.com', 'ipfs.fleek.co' | ||
| ] | ||
|
Comment on lines
+143
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not hardcode domains, especially dead ones like |
||
|
|
||
| const hostname = new URL(gatewayUrl).hostname.toLowerCase() | ||
| if (wellKnownDomains.some(domain => hostname.includes(domain))) { | ||
| score += 10 | ||
| } | ||
|
|
||
| return Math.max(0, Math.min(100, score)) | ||
| } | ||
|
|
||
| /** | ||
| * Check if any hashes from IMG_ARRAY can be loaded from the provided gatewayUrl | ||
| * @param {string} gatewayUrl - The gateway URL to check | ||
|
|
@@ -150,36 +253,72 @@ async function checkViaImgUrl (imgUrl) { | |
| * Checks if a given gateway URL is functioning correctly by verifying image loading and redirection. | ||
| * | ||
| * @param {string} gatewayUrl - The URL of the gateway to be checked. | ||
| * @returns {Promise<boolean>} A promise that resolves to true if the gateway is functioning correctly, otherwise false. | ||
| * @returns {Promise<{isValid: boolean, securityWarnings: string[], errors: string[], securityScore?: number}>} A promise that resolves to an object with validation results including security warnings. | ||
| */ | ||
| export async function checkSubdomainGateway (gatewayUrl) { | ||
| if (gatewayUrl === DEFAULT_SUBDOMAIN_GATEWAY || gatewayUrl === TEST_SUBDOMAIN_GATEWAY) { | ||
| // avoid sending probe requests to the default gateway every time Settings page is opened | ||
| // also skip validation for test gateways | ||
| return true | ||
| return { isValid: true, securityWarnings: [], errors: [] } | ||
| } | ||
|
|
||
| // Perform security validation first | ||
| const securityValidation = validateSubdomainGatewaySecurity(gatewayUrl) | ||
| if (!securityValidation.isValid) { | ||
| return { | ||
| isValid: securityValidation.isValid, | ||
| securityWarnings: securityValidation.warnings, | ||
| errors: securityValidation.errors, | ||
| securityScore: securityValidation.securityScore | ||
| } | ||
| } | ||
| /** @type {URL} */ | ||
| let imgSubdomainUrl | ||
| /** @type {URL} */ | ||
| let imgRedirectedPathUrl | ||
| try { | ||
| const gwUrl = new URL(gatewayUrl) | ||
| imgSubdomainUrl = new URL(`${gwUrl.protocol}//${IMG_HASH_1PX}.ipfs.${gwUrl.hostname}/?now=${Date.now()}&filename=1x1.png#x-ipfs-companion-no-redirect`) | ||
| // Check if the gateway URL already has an IPFS subdomain | ||
| // If hostname starts with 'ipfs.', we need to handle it differently | ||
| const hasIpfsSubdomain = gwUrl.hostname.startsWith('ipfs.') | ||
|
|
||
| if (hasIpfsSubdomain) { | ||
| // For gateways like ipfs.example.com, construct: hash.ipfs.example.com | ||
| imgSubdomainUrl = new URL(`${gwUrl.protocol}//${IMG_HASH_1PX}.${gwUrl.hostname}/?now=${Date.now()}&filename=1x1.png#x-ipfs-companion-no-redirect`) | ||
| } else { | ||
| // For gateways like example.com, construct: hash.ipfs.example.com | ||
| imgSubdomainUrl = new URL(`${gwUrl.protocol}//${IMG_HASH_1PX}.ipfs.${gwUrl.hostname}/?now=${Date.now()}&filename=1x1.png#x-ipfs-companion-no-redirect`) | ||
| } | ||
|
Comment on lines
+286
to
+291
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace entire if with just: The |
||
|
|
||
| imgRedirectedPathUrl = new URL(`${gwUrl.protocol}//${gwUrl.hostname}/ipfs/${IMG_HASH_1PX}?now=${Date.now()}&filename=1x1.png#x-ipfs-companion-no-redirect`) | ||
| } catch (err) { | ||
| console.error('Invalid URL:', err) | ||
| return false | ||
| return { | ||
| isValid: false, | ||
| securityWarnings: securityValidation.warnings, | ||
| errors: ['Invalid URL format: ' + (err instanceof Error ? err.message : String(err))], | ||
| securityScore: securityValidation.securityScore | ||
| } | ||
| } | ||
| try { | ||
| await checkViaImgUrl(imgSubdomainUrl) | ||
| await expectSubdomainRedirect(imgRedirectedPathUrl) | ||
| console.log(`Gateway at '${gatewayUrl}' is functioning correctly (verified image loading and redirection)`) | ||
| return { | ||
| isValid: true, | ||
| securityWarnings: securityValidation.warnings, | ||
| errors: [], | ||
| securityScore: securityValidation.securityScore | ||
| } | ||
| } catch (err) { | ||
| console.error(err) | ||
| return { | ||
| isValid: false, | ||
| securityWarnings: securityValidation.warnings, | ||
| errors: ['Gateway validation failed: ' + (err instanceof Error ? err.message : String(err))], | ||
| securityScore: securityValidation.securityScore | ||
| } | ||
| } | ||
| return await checkViaImgUrl(imgSubdomainUrl) | ||
| .then(async () => expectSubdomainRedirect(imgRedirectedPathUrl)) | ||
| .then(() => { | ||
| console.log(`Gateway at '${gatewayUrl}' is functioning correctly (verified image loading and redirection)`) | ||
| return true | ||
| }) | ||
| .catch((err) => { | ||
| console.error(err) | ||
| return false | ||
| }) | ||
| } | ||
|
|
||
| const bundle = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not block users from being able to use LAN / private network gateways without DNS.
Please remove this