fix: Rate limit is bypassed by sending request header X-Forwarded-For: 127.0.0.1 when Parse Server option trustProxy is permissive

spec/RateLimit.spec.js

@@ -343,6 +343,192 @@ describe('rate limit', () => {
     await Parse.Cloud.run('test2');
   });
 
+  describe('internal request exemption', () => {
+    const middlewares = require('../lib/middlewares');
+    const loginUrl = 'http://localhost:8378/1/login';
+    const loginBody = JSON.stringify({ username: 'someone', password: 'wrong' });
+    const forwardedHeaders = { ...headers, 'X-Forwarded-For': '127.0.0.1' };
+    const loginRateLimit = {
+      requestPath: '/login',
+      requestTimeWindow: 10000,
+      requestCount: 1,
+      errorResponseMessage: 'Too many requests',
+    };
+    const makeFakeReq = ({ remoteAddress, forwardedFor }) => {
+      const fakeReq = {
+        originalUrl: 'http://example.com/parse/login',
+        url: 'http://example.com/login',
+        path: '/login',
+        method: 'POST',
+        ip: '127.0.0.1',
+        socket: { remoteAddress },
+        body: { _ApplicationId: 'test' },
+        headers: {
+          'X-Parse-Application-Id': 'test',
+          'X-Parse-REST-API-Key': 'rest',
+        },
+        get: key => fakeReq.headers[key],
+      };
+      if (forwardedFor !== undefined) {
+        fakeReq.headers['x-forwarded-for'] = forwardedFor;
+      }
+      return fakeReq;
+    };
+    const runThroughRateLimiter = fakeReq =>
+      new Promise(resolve => {
+        const fakeRes = jasmine.createSpyObj('fakeRes', ['end', 'status', 'setHeader']);
+        fakeRes.json = jasmine.createSpy('json').and.callFake(() => resolve('rejected'));
+        middlewares.handleParseHeaders(fakeReq, fakeRes, () => resolve('next'));
+      });
+
+    it('does not exempt a request that presents X-Forwarded-For 127.0.0.1 when trustProxy is true', async () => {
+      await reconfigureServer({ trustProxy: true, rateLimit: [loginRateLimit] });
+      await request({ method: 'POST', headers: forwardedHeaders, url: loginUrl, body: loginBody }).catch(
+        e => e
+      );
+      const response = await request({
+        method: 'POST',
+        headers: forwardedHeaders,
+        url: loginUrl,
+        body: loginBody,
+      }).catch(e => e);
+      expect(response.status).toBe(429);
+      expect(response.data).toEqual({
+        code: Parse.Error.CONNECTION_FAILED,
+        error: 'Too many requests',
+      });
+    });
+
+    it('does not exempt a global zone rate limit for a request that presents X-Forwarded-For 127.0.0.1 when trustProxy is true', async () => {
+      await reconfigureServer({
+        trustProxy: true,
+        rateLimit: [
+          {
+            requestPath: '/classes/*path',
+            requestTimeWindow: 10000,
+            requestCount: 1,
+            errorResponseMessage: 'Too many requests',
+            zone: Parse.Server.RateLimitZone.global,
+          },
+        ],
+      });
+      const url = 'http://localhost:8378/1/classes/MyObject';
+      const body = JSON.stringify({ key: 'value' });
+      await request({ method: 'POST', headers: forwardedHeaders, url, body });
+      const response = await request({ method: 'POST', headers: forwardedHeaders, url, body }).catch(
+        e => e
+      );
+      expect(response.status).toBe(429);
+      expect(response.data).toEqual({
+        code: Parse.Error.CONNECTION_FAILED,
+        error: 'Too many requests',
+      });
+    });
+
+    it('does not exempt batch sub-requests that present X-Forwarded-For 127.0.0.1 when trustProxy is true', async () => {
+      await reconfigureServer({
+        trustProxy: true,
+        rateLimit: [
+          {
+            requestPath: '/classes/*path',
+            requestTimeWindow: 10000,
+            requestCount: 1,
+            errorResponseMessage: 'Too many requests',
+          },
+        ],
+      });
+      const response = await request({
+        method: 'POST',
+        headers: forwardedHeaders,
+        url: 'http://localhost:8378/1/batch',
+        body: JSON.stringify({
+          requests: [
+            { method: 'POST', path: '/1/classes/MyObject', body: { key: 'value1' } },
+            { method: 'POST', path: '/1/classes/MyObject', body: { key: 'value2' } },
+          ],
+        }),
+      }).catch(e => e);
+      expect(response.data).toEqual({
+        code: Parse.Error.CONNECTION_FAILED,
+        error: 'Too many requests',
+      });
+    });
+
+    it('does not exempt a request whose resolved ip is 127.0.0.1 but whose socket peer is remote', async () => {
+      await reconfigureServer({ rateLimit: [loginRateLimit] });
+      const first = await runThroughRateLimiter(
+        makeFakeReq({ remoteAddress: '203.0.113.5', forwardedFor: '127.0.0.1' })
+      );
+      expect(first).toBe('next');
+      const second = await runThroughRateLimiter(
+        makeFakeReq({ remoteAddress: '203.0.113.5', forwardedFor: '127.0.0.1' })
+      );
+      expect(second).toBe('rejected');
+    });
+
+    it('does not exempt a request that presents an empty X-Forwarded-For header when trustProxy is true', async () => {
+      await reconfigureServer({ trustProxy: true, rateLimit: [loginRateLimit] });
+      const emptyForwardedHeaders = { ...headers, 'X-Forwarded-For': '' };
+      await request({
+        method: 'POST',
+        headers: emptyForwardedHeaders,
+        url: loginUrl,
+        body: loginBody,
+      }).catch(e => e);
+      const response = await request({
+        method: 'POST',
+        headers: emptyForwardedHeaders,
+        url: loginUrl,
+        body: loginBody,
+      }).catch(e => e);
+      expect(response.status).toBe(429);
+      expect(response.data).toEqual({
+        code: Parse.Error.CONNECTION_FAILED,
+        error: 'Too many requests',
+      });
+    });
+
+    it('does not exempt a request that arrives over loopback with an empty X-Forwarded-For header', async () => {
+      await reconfigureServer({ rateLimit: [loginRateLimit] });
+      const first = await runThroughRateLimiter(
+        makeFakeReq({ remoteAddress: '127.0.0.1', forwardedFor: '' })
+      );
+      expect(first).toBe('next');
+      const second = await runThroughRateLimiter(
+        makeFakeReq({ remoteAddress: '127.0.0.1', forwardedFor: '' })
+      );
+      expect(second).toBe('rejected');
+    });
+
+    it('does not exempt a request that arrives over loopback with an X-Forwarded-For header', async () => {
+      await reconfigureServer({ rateLimit: [loginRateLimit] });
+      const first = await runThroughRateLimiter(
+        makeFakeReq({ remoteAddress: '127.0.0.1', forwardedFor: '203.0.113.5' })
+      );
+      expect(first).toBe('next');
+      const second = await runThroughRateLimiter(
+        makeFakeReq({ remoteAddress: '127.0.0.1', forwardedFor: '203.0.113.5' })
+      );
+      expect(second).toBe('rejected');
+    });
+
+    it('exempts a request that arrives over loopback without an X-Forwarded-For header when includeInternalRequests is false', async () => {
+      await reconfigureServer({ rateLimit: [loginRateLimit] });
+      const first = await runThroughRateLimiter(makeFakeReq({ remoteAddress: '127.0.0.1' }));
+      expect(first).toBe('next');
+      const second = await runThroughRateLimiter(makeFakeReq({ remoteAddress: '127.0.0.1' }));
+      expect(second).toBe('next');
+    });
+
+    it('does not exempt a request that arrives over loopback without an X-Forwarded-For header when includeInternalRequests is true', async () => {
+      await reconfigureServer({ rateLimit: [{ ...loginRateLimit, includeInternalRequests: true }] });
+      const first = await runThroughRateLimiter(makeFakeReq({ remoteAddress: '127.0.0.1' }));
+      expect(first).toBe('next');
+      const second = await runThroughRateLimiter(makeFakeReq({ remoteAddress: '127.0.0.1' }));
+      expect(second).toBe('rejected');
+    });
+  });
+
   describe('zone', () => {
     const middlewares = require('../lib/middlewares');
     it('can use global zone', async () => {

src/batch.js

@@ -127,7 +127,8 @@ async function handleBatch(router, req) {
         delete info.sessionToken;
       }
       const fakeReq = {
-        ip: req.ip || req.config?.ip || '127.0.0.1',
+        socket: req.socket,
+        headers: req.headers,
         method: (restRequest.method || 'GET').toUpperCase(),
         _batchOriginalMethod: 'POST',
         config: req.config,

src/cloud-code/Parse.Cloud.js

@@ -735,7 +735,7 @@ module.exports = ParseCloud;
  * @property {Boolean} isChallenge If true, means the current request is originally triggered by an auth challenge.
  * @property {Parse.User} user If set, the user that made the request.
  * @property {Parse.Object} object The object triggering the hook.
- * @property {String} ip The IP address of the client making the request. To ensure retrieving the correct IP address, set the Parse Server option `trustProxy: true` if Parse Server runs behind a proxy server, for example behind a load balancer.
+ * @property {String} ip The IP address of the client making the request. If Parse Server runs behind a proxy server, for example a load balancer, set the Parse Server option `trustProxy` to match the proxy setup, for example to the number of proxy hops or to `'loopback'` for a proxy on the same host, so that the address is resolved from the `X-Forwarded-For` header that the proxy sets. Do not set `trustProxy: true`, as that trusts an `X-Forwarded-For` header that any client can set.
  * @property {Object} headers The original HTTP headers for the request.
  * @property {String} triggerName The name of the trigger (`beforeSave`, `afterSave`, ...)
  * @property {Object} log The current logger inside Parse Server.

src/middlewares.js

@@ -662,6 +662,22 @@ export function promiseEnforceMasterKeyAccess(request) {
   return Promise.resolve();
 }
 
+/**
+ * Determines whether a request is an internal request, i.e. one that originates from the same host
+ * without passing through a proxy, such as a Cloud Code request that is routed via HTTP when the
+ * Parse Server option `directAccess` is `false`. Only the raw socket peer address is consulted,
+ * never `request.ip`: under a permissive `trustProxy` setting Express resolves `request.ip` from
+ * the client-supplied `X-Forwarded-For` header, so a remote client could otherwise pose as an
+ * internal request. A request that carries an `X-Forwarded-For` header, even an empty one, has been
+ * forwarded by a proxy or crafted by a client and is therefore never internal, even if it arrives
+ * over a loopback socket.
+ * @param {Object} request The request to evaluate.
+ * @returns {Boolean} Whether the request is internal.
+ */
+const isInternalRequest = request =>
+  request.socket?.remoteAddress === '127.0.0.1' &&
+  request.headers?.['x-forwarded-for'] === undefined;
+
 export const addRateLimit = (route, config, cloud) => {
   if (typeof config === 'string') {
     config = Config.get(config);
@@ -723,7 +739,7 @@ export const addRateLimit = (route, config, cloud) => {
         };
       },
       skip: request => {
-        if (request.ip === '127.0.0.1' && !route.includeInternalRequests) {
+        if (!route.includeInternalRequests && isInternalRequest(request)) {
           return true;
         }
         if (route.includeMasterKey) {