fix(proxy-sigv4-backend): prevent host override (#27)

Signed-off-by: Alec Jacobs <charles.jacobs@segment.com>

Author: alecjacobs5401

plugins/proxy-sigv4-backend/package.json

@@ -45,9 +45,13 @@
     "@backstage/backend-defaults": "^0.11.0",
     "@backstage/backend-test-utils": "^1.6.0",
     "@backstage/cli": "^0.33.0",
+    "@backstage/test-utils": "^1.7.9",
     "@smithy/types": "^3.3.0",
     "@types/aws4": "^1.11.6",
-    "@types/express": "^4.17.20"
+    "@types/express": "^4.17.20",
+    "@types/supertest": "^7.1.0",
+    "msw": "^1.0.0",
+    "supertest": "^7.1.0"
   },
   "files": [
     "dist",

plugins/proxy-sigv4-backend/src/service/router.test.ts

@@ -1,8 +1,15 @@
+import express from 'express';
+import { rest } from 'msw';
+import { setupServer } from 'msw/node';
+import request from 'supertest';
+
 import { mockServices } from '@backstage/backend-test-utils';
+import { registerMswTestHooks } from '@backstage/test-utils';
 
 import {
   buildMiddleware,
   createRouter,
+  joinUrl,
   normalizeRouteConfig,
   normalizeRoutePath,
   credentialsNeedRefresh,
@@ -18,6 +25,28 @@ const mockTemporaryCredentials = jest.fn().mockResolvedValue({
   secretAccessKey: 'SECRET_ACCESS_KEY',
 });
 
+// SSRF_EXAMPLES provides a list of SSRF attack examples and their expected normalized paths
+const SSRF_EXAMPLES = [
+  ['//attacker.example.com/', ''],
+  ['%2F/attacker.example.com/', '%2F/attacker.example.com/'],
+  ['%2F%2Fattacker.example.com/', '%2F%2Fattacker.example.com/'],
+  ['\\attacker.example.com/', 'attacker.example.com/'],
+  ['%5C%5Cattacker.example.com/', '%5C%5Cattacker.example.com/'],
+  ['/%2Fattacker.example.com/', '%2Fattacker.example.com/'],
+  ['//trusted.internal@attacker.example.com/', ''],
+  [
+    '%2F%2Ftrusted.internal%40attacker.example.com/',
+    '%2F%2Ftrusted.internal%40attacker.example.com/',
+  ],
+  ['%252F%252Fattacker.example.com/', '%252F%252Fattacker.example.com/'],
+  [
+    '\uFF0F\uFF0Fattacker.example.com/',
+    '%EF%BC%8F%EF%BC%8Fattacker.example.com/',
+  ],
+  ['/%09/attacker.example.com/', '%09/attacker.example.com/'],
+  ['/%0A/attacker.example.com/', '%0A/attacker.example.com/'],
+];
+
 jest.mock('@aws-sdk/credential-providers', () => ({
   fromNodeProviderChain: jest
     .fn()
@@ -129,6 +158,68 @@ describe('normalizeRouteConfig', () => {
   });
 });
 
+describe('joinUrl', () => {
+  it('joins a request path onto the base URL', () => {
+    expect(joinUrl(new URL('https://example.com'), '/foo').toString()).toBe(
+      'https://example.com/foo',
+    );
+  });
+
+  it('preserves a path prefix on the base URL', () => {
+    expect(joinUrl(new URL('https://example.com/api'), '/foo').toString()).toBe(
+      'https://example.com/api/foo',
+    );
+  });
+
+  it('collapses a trailing slash on the base URL', () => {
+    expect(
+      joinUrl(new URL('https://example.com/api/'), '/foo').toString(),
+    ).toBe('https://example.com/api/foo');
+  });
+
+  it('preserves the query string from the request path', () => {
+    expect(
+      joinUrl(new URL('https://example.com'), '/foo?q=1&r=2').toString(),
+    ).toBe('https://example.com/foo?q=1&r=2');
+  });
+
+  it('rejects a host hijack via protocol-relative path', () => {
+    const joined = joinUrl(new URL('https://example.com'), '//evil.com/foo');
+    expect(joined.host).toBe('example.com');
+    expect(joined.toString()).toBe('https://example.com/foo');
+  });
+
+  it('rejects a host hijack via absolute URL in the request path', () => {
+    const joined = joinUrl(
+      new URL('https://example.com'),
+      'https://evil.com/foo',
+    );
+    expect(joined.host).toBe('example.com');
+    expect(joined.toString()).toBe('https://example.com/foo');
+  });
+
+  it('preserves the configured protocol when the base is http', () => {
+    expect(joinUrl(new URL('http://example.com'), '/foo').toString()).toBe(
+      'http://example.com/foo',
+    );
+  });
+
+  it('handles a root request path', () => {
+    expect(joinUrl(new URL('https://example.com/api'), '/').toString()).toBe(
+      'https://example.com/api/',
+    );
+  });
+
+  it.each(SSRF_EXAMPLES)(
+    'handles a request path that looks like an SSRF attempt: %s',
+    (path, expectedPath) => {
+      const joined = joinUrl(new URL('https://example.com'), path);
+      expect(joined.host).toBe('example.com');
+      expect(joined.toString()).toBe(`https://example.com/${expectedPath}`);
+    },
+  );
+});
+
 describe('credentialsNeedRefresh', () => {
   beforeEach(() => {
     jest.useFakeTimers().setSystemTime(new Date('2024-05-05T12:00:00Z'));
@@ -180,6 +271,10 @@ describe('credentialsNeedRefresh', () => {
 describe('buildMiddleware', () => {
   const logger = mockServices.rootLogger();
 
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
   it('resolves a middleware-like function', async () => {
     const mw = await buildMiddleware({
       logger,
@@ -366,4 +461,163 @@ describe('createRouter', () => {
       expect(router).toBeDefined();
     });
   });
+
+  describe('proxying requests', () => {
+    const app = express();
+    const server = setupServer();
+    registerMswTestHooks(server);
+
+    beforeEach(async () => {
+      const config = mockServices.rootConfig({
+        data: {
+          backend: {
+            baseUrl: 'https://example.com:7007',
+            listen: {
+              port: 7007,
+            },
+          },
+          proxysigv4: {
+            '/test': 'https://example.com',
+          },
+        },
+      });
+      const router = await createRouter({
+        config,
+        logger,
+      });
+      app.use(router);
+    });
+
+    it('proxies requests and returns response from target service', async () => {
+      server.use(
+        rest.get('https://example.com', (_req, res, ctx) => {
+          return res(
+            ctx.status(200),
+            ctx.json({ message: 'Hello from target!' }),
+          );
+        }),
+      );
+
+      const response = await await request(app)
+        .get('/test/')
+        .set('x-msw-bypass', 'true');
+      expect(response.status).toBe(200);
+      expect(response.body).toEqual({ message: 'Hello from target!' });
+    });
+
+    it('proxies requests and forwards params', async () => {
+      expect.assertions(3);
+      server.use(
+        rest.get('https://example.com', (req, res, ctx) => {
+          expect(req.url.searchParams.get('param')).toBe('value');
+          return res(
+            ctx.status(200),
+            ctx.json({ message: 'Hello from target!' }),
+          );
+        }),
+      );
+
+      const response = await await request(app)
+        .get('/test/?param=value')
+        .set('x-msw-bypass', 'true');
+      expect(response.status).toBe(200);
+      expect(response.body).toEqual({ message: 'Hello from target!' });
+    });
+
+    it('proxies handles deep paths', async () => {
+      expect.assertions(3);
+      server.use(
+        rest.get('https://example.com/and/nested/paths', (req, res, ctx) => {
+          expect(req.url.searchParams.get('param')).toBe('value');
+          return res(
+            ctx.status(200),
+            ctx.json({ message: 'Hello from target!' }),
+          );
+        }),
+      );
+
+      const response = await await request(app)
+        .get('/test/and/nested/paths?param=value')
+        .set('x-msw-bypass', 'true');
+      expect(response.status).toBe(200);
+      expect(response.body).toEqual({ message: 'Hello from target!' });
+    });
+
+    it('does not allow for ssrf', async () => {
+      server.use(
+        rest.get('https://example.com/', (_req, res, ctx) => {
+          return res(
+            ctx.status(200),
+            ctx.json({ message: 'Hello from target!' }),
+          );
+        }),
+      );
+
+      const response = await await request(app)
+        .get('/test////other.domain.com')
+        .set('x-msw-bypass', 'true');
+      expect(response.status).toBe(200);
+      expect(response.body).toEqual({ message: 'Hello from target!' });
+    });
+
+    it.each(SSRF_EXAMPLES)(
+      'does not allow for ssrf: %s',
+      async (path, expectedPath) => {
+        server.use(
+          rest.get('https://example.com/*', (req, res, ctx) => {
+            return res(
+              ctx.status(404),
+              ctx.json({ message: `Naughty path! ${req.url.pathname}` }),
+            );
+          }),
+        );
+
+        const response = await await request(app)
+          .get(`/test/${path}`)
+          .set('x-msw-bypass', 'true');
+        expect(response.status).toBe(404);
+        expect(response.body).toEqual({
+          message: `Naughty path! /${expectedPath}`,
+        });
+      },
+    );
+
+    it('normalizes backslashes in the request path', async () => {
+      server.use(
+        rest.get('https://example.com/127.0.0.1:3007', (_req, res, ctx) => {
+          return res(
+            ctx.status(200),
+            ctx.json({ message: 'Hello from target!' }),
+          );
+        }),
+      );
+
+      const response = await await request(app)
+        .get('/test/\\127.0.0.1:3007')
+        .set('x-msw-bypass', 'true');
+
+      expect(response.status).toBe(200);
+      expect(response.body).toEqual({ message: 'Hello from target!' });
+    });
+
+    it('allows valid query params', async () => {
+      expect.assertions(3);
+      server.use(
+        rest.get('https://example.com/', (req, res, ctx) => {
+          expect(req.url.searchParams.get('q')).toBe('///some.other.domain');
+          return res(
+            ctx.status(200),
+            ctx.json({ message: 'Hello from target!' }),
+          );
+        }),
+      );
+
+      const response = await await request(app)
+        .get('/test?q=///some.other.domain')
+        .set('x-msw-bypass', 'true');
+
+      expect(response.status).toBe(200);
+      expect(response.body).toEqual({ message: 'Hello from target!' });
+    });
+  });
 });

plugins/proxy-sigv4-backend/src/service/router.ts

@@ -119,6 +119,24 @@ export const credentialsNeedRefresh = (
   credentials.expiration.getTime() - Date.now() <
     CREDENTIAL_NEED_REFRESH_BUFFER;
 
+/**
+ * Joins a request path onto a configured target URL while pinning protocol
+ * and host to the target. Only `.pathname` and `.search` are read from the
+ * parsed request URL, so an attacker-supplied `//evil.com/...` or
+ * `https://evil.com/...` cannot redirect the upstream call.
+ *
+ * @internal
+ */
+export function joinUrl(base: URL, requestPath: string): URL {
+  const incoming = new URL(requestPath, 'http://placeholder.invalid');
+  const basePath = base.pathname.replace(/\/+$/, '');
+  const incomingPath = incoming.pathname.replace(/^\/+/, '/');
+  const joined = new URL(base.toString());
+  joined.pathname = basePath + incomingPath;
+  joined.search = incoming.search;
+  return joined;
+}
+
 /** @internal */
 export async function buildMiddleware(
   options: MiddlewareOptions,
@@ -189,14 +207,14 @@ export async function buildMiddleware(
   ) => {
     try {
       const requestHeaders = filterHeaders(req.headers as HeadersMap);
-      const targetUrl = new URL(req.url, target);
+      const targetUrl = joinUrl(new URL(target), req.url);
 
       // request is provided to aws4.sign() and mutated in place for new headers
       const request: any = {
         method: req.method,
         protocol: targetUrl.protocol,
         host: targetUrl.host,
-        path: req.url, // path + search
+        path: targetUrl.pathname + targetUrl.search,
         headers: requestHeaders,
         service: service,
         region: region,