Skip to content

Commit

Permalink
fix: The client IP address may be determined incorrectly in some case…
Browse files Browse the repository at this point in the history
…s; it is now required to set the Parse Server option `trustProxy` accordingly if Parse Server runs behind a proxy server, see the express framework's [trust proxy](https://expressjs.com/en/guide/behind-proxies.html) setting; this fixes a security vulnerability in which the Parse Server option `masterKeyIps` may be circumvented, see [GHSA-vm5r-c87r-pf6x](GHSA-vm5r-c87r-pf6x) (#8369)
  • Loading branch information
mtrezza committed Jan 5, 2023
1 parent c8bc200 commit e016d81
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 134 deletions.
121 changes: 3 additions & 118 deletions spec/Middlewares.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,78 +158,6 @@ describe('middlewares', () => {
});
});

it('should not succeed if the connection.remoteAddress does not belong to masterKeyIps list', () => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1', 'ip2'],
});
fakeReq.connection = { remoteAddress: 'ip3' };
fakeReq.headers['x-parse-master-key'] = 'masterKey';
middlewares.handleParseHeaders(fakeReq, fakeRes);
expect(fakeRes.status).toHaveBeenCalledWith(403);
});

it('should succeed if the connection.remoteAddress does belong to masterKeyIps list', done => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1', 'ip2'],
});
fakeReq.connection = { remoteAddress: 'ip1' };
fakeReq.headers['x-parse-master-key'] = 'masterKey';
middlewares.handleParseHeaders(fakeReq, fakeRes, () => {
expect(fakeRes.status).not.toHaveBeenCalled();
done();
});
});

it('should not succeed if the socket.remoteAddress does not belong to masterKeyIps list', () => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1', 'ip2'],
});
fakeReq.socket = { remoteAddress: 'ip3' };
fakeReq.headers['x-parse-master-key'] = 'masterKey';
middlewares.handleParseHeaders(fakeReq, fakeRes);
expect(fakeRes.status).toHaveBeenCalledWith(403);
});

it('should succeed if the socket.remoteAddress does belong to masterKeyIps list', done => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1', 'ip2'],
});
fakeReq.socket = { remoteAddress: 'ip1' };
fakeReq.headers['x-parse-master-key'] = 'masterKey';
middlewares.handleParseHeaders(fakeReq, fakeRes, () => {
expect(fakeRes.status).not.toHaveBeenCalled();
done();
});
});

it('should not succeed if the connection.socket.remoteAddress does not belong to masterKeyIps list', () => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1', 'ip2'],
});
fakeReq.connection = { socket: { remoteAddress: 'ip3' } };
fakeReq.headers['x-parse-master-key'] = 'masterKey';
middlewares.handleParseHeaders(fakeReq, fakeRes);
expect(fakeRes.status).toHaveBeenCalledWith(403);
});

it('should succeed if the connection.socket.remoteAddress does belong to masterKeyIps list', done => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1', 'ip2'],
});
fakeReq.connection = { socket: { remoteAddress: 'ip1' } };
fakeReq.headers['x-parse-master-key'] = 'masterKey';
middlewares.handleParseHeaders(fakeReq, fakeRes, () => {
expect(fakeRes.status).not.toHaveBeenCalled();
done();
});
});

it('should allow any ip to use masterKey if masterKeyIps is empty', done => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
Expand All @@ -243,52 +171,9 @@ describe('middlewares', () => {
});
});

it('should succeed if xff header does belong to masterKeyIps', done => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1'],
});
fakeReq.headers['x-parse-master-key'] = 'masterKey';
fakeReq.headers['x-forwarded-for'] = 'ip1, ip2, ip3';
middlewares.handleParseHeaders(fakeReq, fakeRes, () => {
expect(fakeRes.status).not.toHaveBeenCalled();
done();
});
});

it('should succeed if xff header with one ip does belong to masterKeyIps', done => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1'],
});
fakeReq.headers['x-parse-master-key'] = 'masterKey';
fakeReq.headers['x-forwarded-for'] = 'ip1';
middlewares.handleParseHeaders(fakeReq, fakeRes, () => {
expect(fakeRes.status).not.toHaveBeenCalled();
done();
});
});

it('should not succeed if xff header does not belong to masterKeyIps', () => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip4'],
});
fakeReq.headers['x-parse-master-key'] = 'masterKey';
fakeReq.headers['x-forwarded-for'] = 'ip1, ip2, ip3';
middlewares.handleParseHeaders(fakeReq, fakeRes);
expect(fakeRes.status).toHaveBeenCalledWith(403);
});

it('should not succeed if xff header is empty and masterKeyIps is set', () => {
AppCache.put(fakeReq.body._ApplicationId, {
masterKey: 'masterKey',
masterKeyIps: ['ip1'],
});
fakeReq.headers['x-parse-master-key'] = 'masterKey';
fakeReq.headers['x-forwarded-for'] = '';
middlewares.handleParseHeaders(fakeReq, fakeRes);
expect(fakeRes.status).toHaveBeenCalledWith(403);
it('can set trust proxy', async () => {
const server = await reconfigureServer({ trustProxy: 1 });
expect(server.app.parent.settings['trust proxy']).toBe(1);
});

it('should properly expose the headers', () => {
Expand Down
7 changes: 7 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,13 @@ module.exports.ParseServerOptions = {
help: 'Starts the liveQuery server',
action: parsers.booleanParser,
},
trustProxy: {
env: 'PARSE_SERVER_TRUST_PROXY',
help:
'The trust proxy settings. It is important to understand the exact setup of the reverse proxy, since this setting will trust values provided in the Parse Server API request. See the <a href="https://expressjs.com/en/guide/behind-proxies.html">express trust proxy settings</a> documentation. Defaults to `false`.',
action: parsers.objectParser,
default: [],
},
userSensitiveFields: {
env: 'PARSE_SERVER_USER_SENSITIVE_FIELDS',
help:
Expand Down
1 change: 1 addition & 0 deletions src/Options/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
* @property {Number} sessionLength Session duration, in seconds, defaults to 1 year
* @property {Boolean} silent Disables console output
* @property {Boolean} startLiveQueryServer Starts the liveQuery server
* @property {Any} trustProxy The trust proxy settings. It is important to understand the exact setup of the reverse proxy, since this setting will trust values provided in the Parse Server API request. See the <a href="https://expressjs.com/en/guide/behind-proxies.html">express trust proxy settings</a> documentation. Defaults to `false`.
* @property {String[]} userSensitiveFields Personally identifiable information fields in the user table the should be removed for non-authorized users. Deprecated @see protectedFields
* @property {Boolean} verbose Set the logging to verbose
* @property {Boolean} verifyUserEmails Set to `true` to require users to verify their email address to complete the sign-up process.<br><br>Default is `false`.
Expand Down
3 changes: 3 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ export interface ParseServerOptions {
cluster: ?NumberOrBoolean;
/* middleware for express server, can be string or function */
middleware: ?((() => void) | string);
/* The trust proxy settings. It is important to understand the exact setup of the reverse proxy, since this setting will trust values provided in the Parse Server API request. See the <a href="https://expressjs.com/en/guide/behind-proxies.html">express trust proxy settings</a> documentation. Defaults to `false`.
:DEFAULT: false */
trustProxy: ?any;
/* Starts the liveQuery server */
startLiveQueryServer: ?boolean;
/* Live query server configuration options (will start the liveQuery server) */
Expand Down
3 changes: 3 additions & 0 deletions src/ParseServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ class ParseServer {
options
);
}
if (options.trustProxy) {
app.set('trust proxy', options.trustProxy);
}
/* istanbul ignore next */
if (!process.env.TESTING) {
configureListeners(this);
Expand Down
17 changes: 1 addition & 16 deletions src/middlewares.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,22 +280,7 @@ export function handleParseHeaders(req, res, next) {
}

function getClientIp(req) {
if (req.headers['x-forwarded-for']) {
// try to get from x-forwared-for if it set (behind reverse proxy)
return req.headers['x-forwarded-for'].split(',')[0];
} else if (req.connection && req.connection.remoteAddress) {
// no proxy, try getting from connection.remoteAddress
return req.connection.remoteAddress;
} else if (req.socket) {
// try to get it from req.socket
return req.socket.remoteAddress;
} else if (req.connection && req.connection.socket) {
// try to get it form the connection.socket
return req.connection.socket.remoteAddress;
} else {
// if non above, fallback.
return req.ip;
}
return req.ip;
}

function httpAuth(req) {
Expand Down

0 comments on commit e016d81

Please sign in to comment.