From 0ee35d26f2002ccef84ead30cf6cc2a851ca1c60 Mon Sep 17 00:00:00 2001 From: Nishant Arora <1895906+whizzzkid@users.noreply.github.com> Date: Wed, 22 Feb 2023 11:53:50 -0700 Subject: [PATCH] fix(recovery): :bug: false-positive for non-gateway URLs (#1163) * fix(recovery): :bug: No recovery for URLs being redirected. * fix(recovery): :test_tube: test added. * fix(recovery): :memo: Adding comments * fix(recovery): :rotating_light: linter warnings * fix: false-positives in sameGateway This ensures sameGateway returns true only if the tested URL is either a valid path, subdomain, or RPC URL related to local Gateway or Kubo instance. Everything else should be ignored, because people may run Kubo Gateway on localhost:8080, and then stop it, and then run some unrelated HTTP server on the same port. This is unfortunate papercut due to the 8080 being very very popular default port for all things HTTP. Luckily, Companion has enough information to make this right. * test: ensure recovery does not depend on redirects --------- Co-authored-by: Marcin Rataj --- add-on/src/lib/ipfs-path.js | 5 +++ test/functional/lib/ipfs-path.test.js | 16 ++++++++-- .../lib/ipfs-request-gateway-redirect.test.js | 31 ++++++++++++++++--- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/add-on/src/lib/ipfs-path.js b/add-on/src/lib/ipfs-path.js index 2490d92ae..06b012755 100644 --- a/add-on/src/lib/ipfs-path.js +++ b/add-on/src/lib/ipfs-path.js @@ -126,6 +126,11 @@ export function sameGateway (url, gwUrl) { url.hostname = '127.0.0.1' } + // Additional check to avoid false-positives when user has some unrelated HTTP server running on localhost:8080 + // It is not "sameGateway" if "localhost" URL does not look like Gateway or RPC URL. + // This removes surface for bugs like https://github.com/ipfs/ipfs-companion/issues/1162 + if (!(isIPFS.url(url.toString()) || isIPFS.subdomain(url.toString()) || url.pathname.startsWith('/api/v0/') || url.pathname.startsWith('/webui'))) return false + const gws = [gwUrl.host] // localhost gateway has more than one hostname diff --git a/test/functional/lib/ipfs-path.test.js b/test/functional/lib/ipfs-path.test.js index cbca12408..c72eea739 100644 --- a/test/functional/lib/ipfs-path.test.js +++ b/test/functional/lib/ipfs-path.test.js @@ -184,11 +184,23 @@ describe('ipfs-path.js', function () { const gw = 'http://localhost:8080' expect(sameGateway(url, gw)).to.equal(true) }) - it('should return true on 127.0.0.1/0.0.0.0 host match', function () { + it('should return true on localhost subdomain host match', function () { + const url = 'http://bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi.ipfs.localhost:8080/foo/bar' + const gw = 'http://localhost:8080' + expect(sameGateway(url, gw)).to.equal(true) + }) + it('should return true on RPC webui path 127.0.0.1/0.0.0.0 host match', function () { + // this is misconfiguration, but handling it in sameGateway ensures users who do this dont waste our time debugging const url = 'http://0.0.0.0:5001/webui' const api = 'http://127.0.0.1:5001' expect(sameGateway(url, api)).to.equal(true) }) + it('should return true on RPC /api/v0 path 127.0.0.1/0.0.0.0 host match', function () { + // this is misconfiguration, but handling it in sameGateway ensures users who do this dont waste our time debugging + const url = 'http://0.0.0.0:5001/api/v0/id' + const api = 'http://127.0.0.1:5001' + expect(sameGateway(url, api)).to.equal(true) + }) it('should return false on hostname match but different port', function () { const url = 'https://localhost:8081/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar' const gw = 'http://localhost:8080' @@ -289,7 +301,7 @@ describe('ipfs-path.js', function () { expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true) }) it('should return false for /ipfs/ URL at Local Gateway', function () { - const url = `${state.gwURL}/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest` + const url = `${state.gwURL}ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest` expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false) }) it('should return false for IPFS content loaded from IPFS API port', function () { diff --git a/test/functional/lib/ipfs-request-gateway-redirect.test.js b/test/functional/lib/ipfs-request-gateway-redirect.test.js index 07d3ccacc..655fe49c0 100644 --- a/test/functional/lib/ipfs-request-gateway-redirect.test.js +++ b/test/functional/lib/ipfs-request-gateway-redirect.test.js @@ -353,7 +353,7 @@ describe('modifyRequest.onBeforeRequest:', function () { describe('request for IPFS path at the localhost', function () { // we do not touch local requests, as it may interfere with other nodes running at the same machine // or could produce false-positives such as redirection from localhost:5001/ipfs/path to localhost:8080/ipfs/path - it('should fix localhost API hostname to IP', function () { + it('should fix localhost Kubo RPC hostname to IP', function () { const request = url2request('http://localhost:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') // expectNoRedirect(modifyRequest, request) expect(modifyRequest.onBeforeRequest(request).redirectUrl) @@ -375,14 +375,14 @@ describe('modifyRequest.onBeforeRequest:', function () { expect(modifyRequest.onBeforeRequest(request).redirectUrl) .to.equal('http://127.0.0.1:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') }) - it('should fix localhost API to IP', function () { + it('should be left untouched if /webui on localhost Kubo RPC port', function () { // https://github.com/ipfs/ipfs-companion/issues/291 const request = url2request('http://localhost:5001/webui') // expectNoRedirect(modifyRequest, request) expect(modifyRequest.onBeforeRequest(request).redirectUrl) .to.equal('http://127.0.0.1:5001/webui') }) - it('should be left untouched if localhost API IP is used, even when x-ipfs-path is present', function () { + it('should be left untouched if localhost Kubo RPC IP is used, even when x-ipfs-path is present', function () { // https://github.com/ipfs-shipyard/ipfs-companion/issues/604 const request = url2request('http://127.0.0.1:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') request.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DDIFF' }] @@ -431,17 +431,38 @@ describe('modifyRequest.onBeforeRequest:', function () { global.browser = browser state.ipfsNodeType = 'external' state.redirect = true - state.peerCount = -1 + state.peerCount = -1 // this simulates Kubo RPC being offline state.gwURLString = 'http://localhost:8080' state.gwURL = new URL('http://localhost:8080') state.pubGwURLString = 'https://ipfs.io' state.pubGwURL = new URL('https://ipfs.io') }) - it('should present recovery page if node is offline', function () { + it('should present recovery page if node is offline and redirect is enabled', function () { expect(state.nodeActive).to.be.equal(false) + state.redirect = true + const request = url2request('https://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar') + expect(modifyRequest.onBeforeRequest(request).redirectUrl).to.equal('chrome-extension://testid/dist/recovery/recovery.html#https%3A%2F%2Fipfs.io%2Fipfs%2FQmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR%2Ffoo%2Fbar') + }) + it('should present recovery page if node is offline and redirect is disabled', function () { + expect(state.nodeActive).to.be.equal(false) + state.redirect = false const request = url2request('https://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar') expect(modifyRequest.onBeforeRequest(request).redirectUrl).to.equal('chrome-extension://testid/dist/recovery/recovery.html#https%3A%2F%2Fipfs.io%2Fipfs%2FQmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR%2Ffoo%2Fbar') }) + it('should not show recovery page if node is offline, redirect is enabled, but non-gateway URL failed to load from the same port', function () { + // this covers https://github.com/ipfs/ipfs-companion/issues/1162 and https://twitter.com/unicomp21/status/1626244123102679041 + state.redirect = true + expect(state.nodeActive).to.be.equal(false) + const request = url2request('https://localhost:8080/') + expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + }) + it('should not show recovery page if extension is disabled', function () { + // allows user to quickly avoid anything similar to https://github.com/ipfs/ipfs-companion/issues/1162 + state.active = false + expect(state.nodeActive).to.be.equal(false) + const request = url2request('https://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar') + expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + }) }) after(function () {