fix relative urls in external url service (#116404)

elastic · Nov 2, 2021 · 14c0d35 · 14c0d35
1 parent b03a869
commit 14c0d35
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 4 deletions.
diff --git a/src/core/public/http/external_url_service.test.ts b/src/core/public/http/external_url_service.test.ts
@@ -155,6 +155,40 @@ describe('External Url Service', () => {
         });
       });
 
+      internalRequestScenarios.forEach(({ description, policy, allowExternal }) => {
+        describe(description, () => {
+          it('allows relative URLs without absolute path replacing last path segment', () => {
+            const { setup } = setupService({ location, serverBasePath, policy });
+            const urlCandidate = `my_other_app?foo=bar`;
+            const result = setup.validateUrl(urlCandidate);
+
+            expect(result).toBeInstanceOf(URL);
+            expect(result?.toString()).toEqual(`${kibanaRoot}/app/${urlCandidate}`);
+          });
+
+          it('allows relative URLs without absolute path replacing multiple path segments', () => {
+            const { setup } = setupService({ location, serverBasePath, policy });
+            const urlCandidate = `/api/my_other_app?foo=bar`;
+            const result = setup.validateUrl(`..${urlCandidate}`);
+
+            expect(result).toBeInstanceOf(URL);
+            expect(result?.toString()).toEqual(`${kibanaRoot}${urlCandidate}`);
+          });
+
+          if (!allowExternal) {
+            describe('handles bypass of base path via relative URL', () => {
+              it('does not allow relative URLs that escape base path', () => {
+                const { setup } = setupService({ location, serverBasePath, policy: [] });
+                const urlCandidate = `../../base_path_escape`;
+                const result = setup.validateUrl(urlCandidate);
+
+                expect(result).toBeNull();
+              });
+            });
+          }
+        });
+      });
+
       describe('handles protocol resolution bypass', () => {
         it('does not allow relative URLs that include a host', () => {
           const { setup } = setupService({ location, serverBasePath, policy: [] });
@@ -194,7 +228,7 @@ describe('External Url Service', () => {
 
       internalRequestScenarios.forEach(({ description, policy }) => {
         describe(description, () => {
-          it('allows relative URLs', () => {
+          it('allows relative URLs with absolute path', () => {
             const { setup } = setupService({ location, serverBasePath, policy });
             const urlCandidate = `/some/path?foo=bar`;
             const result = setup.validateUrl(`${serverBasePath}${urlCandidate}`);
@@ -214,6 +248,28 @@ describe('External Url Service', () => {
         });
       });
 
+      internalRequestScenarios.forEach(({ description, policy }) => {
+        describe(description, () => {
+          it('allows relative URLs without absolute path replacing last path segment', () => {
+            const { setup } = setupService({ location, serverBasePath, policy });
+            const urlCandidate = `my_other_app?foo=bar`;
+            const result = setup.validateUrl(urlCandidate);
+
+            expect(result).toBeInstanceOf(URL);
+            expect(result?.toString()).toEqual(`${kibanaRoot}/app/${urlCandidate}`);
+          });
+
+          it('allows relative URLs without absolute path replacing multiple path segments', () => {
+            const { setup } = setupService({ location, serverBasePath, policy });
+            const urlCandidate = `/api/my_other_app?foo=bar`;
+            const result = setup.validateUrl(`..${urlCandidate}`);
+
+            expect(result).toBeInstanceOf(URL);
+            expect(result?.toString()).toEqual(`${kibanaRoot}${urlCandidate}`);
+          });
+        });
+      });
+
       describe('handles protocol resolution bypass', () => {
         it('does not allow relative URLs that include a host', () => {
           const { setup } = setupService({ location, serverBasePath, policy: [] });

diff --git a/src/core/public/http/external_url_service.ts b/src/core/public/http/external_url_service.ts
@@ -14,7 +14,7 @@ import { InjectedMetadataSetup } from '../injected_metadata';
 import { Sha256 } from '../utils';
 
 interface SetupDeps {
-  location: Pick<Location, 'origin'>;
+  location: Pick<Location, 'href'>;
   injectedMetadata: InjectedMetadataSetup;
 }
 
@@ -52,11 +52,11 @@ function normalizeProtocol(protocol: string) {
 
 const createExternalUrlValidation = (
   rules: IExternalUrlPolicy[],
-  location: Pick<Location, 'origin'>,
+  location: Pick<Location, 'href'>,
   serverBasePath: string
 ) => {
-  const base = new URL(location.origin + serverBasePath);
   return function validateExternalUrl(next: string) {
+    const base = new URL(location.href);
     const url = new URL(next, base);
 
     const isInternalURL =