Skip to content

Commit b43b224

Browse files
mtrezzamtrezza
authored
SQL injection via query field name when using PostgreSQL ([GHSA-c442-97qw-j6c6](GHSA-c442-97qw-j6c6)) (#10177)
1 parent c795c14 commit b43b224

File tree

3 files changed

+322
-30
lines changed

3 files changed

+322
-30
lines changed

‎spec/vulnerabilities.spec.js‎

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,3 +2112,258 @@ describe('(GHSA-w54v-hf9p-8856) User enumeration via email verification endpoint
21122112
}
21132113
});
21142114
});
2115+
2116+
describe('(GHSA-c442-97qw-j6c6) SQL Injection via $regex query operator field name in PostgreSQL adapter', () => {
2117+
const headers = {
2118+
'Content-Type': 'application/json',
2119+
'X-Parse-Application-Id': 'test',
2120+
'X-Parse-REST-API-Key': 'rest',
2121+
'X-Parse-Master-Key': 'test',
2122+
};
2123+
const serverURL = 'http://localhost:8378/1';
2124+
2125+
beforeEach(async () => {
2126+
const obj = new Parse.Object('TestClass');
2127+
obj.set('playerName', 'Alice');
2128+
obj.set('score', 100);
2129+
await obj.save(null, { useMasterKey: true });
2130+
});
2131+
2132+
it('rejects field names containing double quotes in $regex query with master key', async () => {
2133+
const maliciousField = 'playerName" OR 1=1 --';
2134+
const response = await request({
2135+
method: 'GET',
2136+
url: `${serverURL}/classes/TestClass`,
2137+
headers,
2138+
qs: {
2139+
where: JSON.stringify({
2140+
[maliciousField]: { $regex: 'x' },
2141+
}),
2142+
},
2143+
}).catch(e => e);
2144+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2145+
});
2146+
2147+
it('rejects field names containing single quotes in $regex query with master key', async () => {
2148+
const maliciousField = "playerName' OR '1'='1";
2149+
const response = await request({
2150+
method: 'GET',
2151+
url: `${serverURL}/classes/TestClass`,
2152+
headers,
2153+
qs: {
2154+
where: JSON.stringify({
2155+
[maliciousField]: { $regex: 'x' },
2156+
}),
2157+
},
2158+
}).catch(e => e);
2159+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2160+
});
2161+
2162+
it('rejects field names containing semicolons in $regex query with master key', async () => {
2163+
const maliciousField = 'playerName; DROP TABLE "TestClass" --';
2164+
const response = await request({
2165+
method: 'GET',
2166+
url: `${serverURL}/classes/TestClass`,
2167+
headers,
2168+
qs: {
2169+
where: JSON.stringify({
2170+
[maliciousField]: { $regex: 'x' },
2171+
}),
2172+
},
2173+
}).catch(e => e);
2174+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2175+
});
2176+
2177+
it('rejects field names containing parentheses in $regex query with master key', async () => {
2178+
const maliciousField = 'playerName" ~ \'x\' OR (SELECT 1) --';
2179+
const response = await request({
2180+
method: 'GET',
2181+
url: `${serverURL}/classes/TestClass`,
2182+
headers,
2183+
qs: {
2184+
where: JSON.stringify({
2185+
[maliciousField]: { $regex: 'x' },
2186+
}),
2187+
},
2188+
}).catch(e => e);
2189+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2190+
});
2191+
2192+
it('allows legitimate $regex query with master key', async () => {
2193+
const response = await request({
2194+
method: 'GET',
2195+
url: `${serverURL}/classes/TestClass`,
2196+
headers,
2197+
qs: {
2198+
where: JSON.stringify({
2199+
playerName: { $regex: 'Ali' },
2200+
}),
2201+
},
2202+
});
2203+
expect(response.data.results.length).toBe(1);
2204+
expect(response.data.results[0].playerName).toBe('Alice');
2205+
});
2206+
2207+
it('allows legitimate $regex query with dot notation and master key', async () => {
2208+
const obj = new Parse.Object('TestClass');
2209+
obj.set('metadata', { tag: 'hello-world' });
2210+
await obj.save(null, { useMasterKey: true });
2211+
const response = await request({
2212+
method: 'GET',
2213+
url: `${serverURL}/classes/TestClass`,
2214+
headers,
2215+
qs: {
2216+
where: JSON.stringify({
2217+
'metadata.tag': { $regex: 'hello' },
2218+
}),
2219+
},
2220+
});
2221+
expect(response.data.results.length).toBe(1);
2222+
expect(response.data.results[0].metadata.tag).toBe('hello-world');
2223+
});
2224+
2225+
it('allows legitimate $regex query without master key', async () => {
2226+
const response = await request({
2227+
method: 'GET',
2228+
url: `${serverURL}/classes/TestClass`,
2229+
headers: {
2230+
'Content-Type': 'application/json',
2231+
'X-Parse-Application-Id': 'test',
2232+
'X-Parse-REST-API-Key': 'rest',
2233+
},
2234+
qs: {
2235+
where: JSON.stringify({
2236+
playerName: { $regex: 'Ali' },
2237+
}),
2238+
},
2239+
});
2240+
expect(response.data.results.length).toBe(1);
2241+
expect(response.data.results[0].playerName).toBe('Alice');
2242+
});
2243+
2244+
it('rejects field names with SQL injection via non-$regex operators with master key', async () => {
2245+
const maliciousField = 'playerName" OR 1=1 --';
2246+
const response = await request({
2247+
method: 'GET',
2248+
url: `${serverURL}/classes/TestClass`,
2249+
headers,
2250+
qs: {
2251+
where: JSON.stringify({
2252+
[maliciousField]: { $exists: true },
2253+
}),
2254+
},
2255+
}).catch(e => e);
2256+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2257+
});
2258+
2259+
describe('validateQuery key name enforcement', () => {
2260+
const maliciousField = 'field"; DROP TABLE test --';
2261+
const noMasterHeaders = {
2262+
'Content-Type': 'application/json',
2263+
'X-Parse-Application-Id': 'test',
2264+
'X-Parse-REST-API-Key': 'rest',
2265+
};
2266+
2267+
it('rejects malicious field name in find without master key', async () => {
2268+
const response = await request({
2269+
method: 'GET',
2270+
url: `${serverURL}/classes/TestClass`,
2271+
headers: noMasterHeaders,
2272+
qs: {
2273+
where: JSON.stringify({ [maliciousField]: 'value' }),
2274+
},
2275+
}).catch(e => e);
2276+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2277+
});
2278+
2279+
it('rejects malicious field name in find with master key', async () => {
2280+
const response = await request({
2281+
method: 'GET',
2282+
url: `${serverURL}/classes/TestClass`,
2283+
headers,
2284+
qs: {
2285+
where: JSON.stringify({ [maliciousField]: 'value' }),
2286+
},
2287+
}).catch(e => e);
2288+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2289+
});
2290+
2291+
it('allows master key to query whitelisted internal field _email_verify_token', async () => {
2292+
await reconfigureServer({
2293+
verifyUserEmails: true,
2294+
emailAdapter: {
2295+
sendVerificationEmail: () => Promise.resolve(),
2296+
sendPasswordResetEmail: () => Promise.resolve(),
2297+
sendMail: () => {},
2298+
},
2299+
appName: 'test',
2300+
publicServerURL: 'http://localhost:8378/1',
2301+
});
2302+
const user = new Parse.User();
2303+
user.setUsername('testuser');
2304+
user.setPassword('testpass');
2305+
user.setEmail('test@example.com');
2306+
await user.signUp();
2307+
const response = await request({
2308+
method: 'GET',
2309+
url: `${serverURL}/classes/_User`,
2310+
headers,
2311+
qs: {
2312+
where: JSON.stringify({ _email_verify_token: { $exists: true } }),
2313+
},
2314+
});
2315+
expect(response.data.results.length).toBeGreaterThan(0);
2316+
});
2317+
2318+
it('rejects non-master key querying internal field _email_verify_token', async () => {
2319+
const response = await request({
2320+
method: 'GET',
2321+
url: `${serverURL}/classes/_User`,
2322+
headers: noMasterHeaders,
2323+
qs: {
2324+
where: JSON.stringify({ _email_verify_token: { $exists: true } }),
2325+
},
2326+
}).catch(e => e);
2327+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2328+
});
2329+
2330+
describe('non-master key cannot update internal fields', () => {
2331+
const internalFields = [
2332+
'_rperm',
2333+
'_wperm',
2334+
'_hashed_password',
2335+
'_email_verify_token',
2336+
'_perishable_token',
2337+
'_perishable_token_expires_at',
2338+
'_email_verify_token_expires_at',
2339+
'_failed_login_count',
2340+
'_account_lockout_expires_at',
2341+
'_password_changed_at',
2342+
'_password_history',
2343+
'_tombstone',
2344+
'_session_token',
2345+
];
2346+
2347+
for (const field of internalFields) {
2348+
it(`rejects non-master key updating ${field}`, async () => {
2349+
const user = new Parse.User();
2350+
user.setUsername(`updatetest_${field}`);
2351+
user.setPassword('password123');
2352+
await user.signUp();
2353+
const response = await request({
2354+
method: 'PUT',
2355+
url: `${serverURL}/classes/_User/${user.id}`,
2356+
headers: {
2357+
'Content-Type': 'application/json',
2358+
'X-Parse-Application-Id': 'test',
2359+
'X-Parse-REST-API-Key': 'rest',
2360+
'X-Parse-Session-Token': user.getSessionToken(),
2361+
},
2362+
body: JSON.stringify({ [field]: 'malicious_value' }),
2363+
}).catch(e => e);
2364+
expect(response.data.code).toBe(Parse.Error.INVALID_KEY_NAME);
2365+
});
2366+
}
2367+
});
2368+
});
2369+
});

‎src/Adapters/Storage/Postgres/PostgresStorageAdapter.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ const transformDotFieldToComponents = fieldName => {
225225

226226
const transformDotField = fieldName => {
227227
if (fieldName.indexOf('.') === -1) {
228-
return `"${fieldName}"`;
228+
return `"${fieldName.replace(/"/g, '""')}"`;
229229
}
230230
const components = transformDotFieldToComponents(fieldName);
231231
let name = components.slice(0, components.length - 1).join('->');
@@ -760,11 +760,16 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus
760760
}
761761
}
762762

763-
const name = transformDotField(fieldName);
764763
regex = processRegexPattern(regex);
765764

766-
patterns.push(`$${index}:raw ${operator} '$${index + 1}:raw'`);
767-
values.push(name, regex);
765+
if (fieldName.indexOf('.') >= 0) {
766+
const name = transformDotField(fieldName);
767+
patterns.push(`$${index}:raw ${operator} '$${index + 1}:raw'`);
768+
values.push(name, regex);
769+
} else {
770+
patterns.push(`$${index}:name ${operator} '$${index + 1}:raw'`);
771+
values.push(fieldName, regex);
772+
}
768773
index += 2;
769774
}
770775

‎src/Controllers/DatabaseController.js‎

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,61 @@ import type { ParseServerOptions } from '../Options';
2222
import type { QueryOptions, FullQueryOptions } from '../Adapters/Storage/StorageAdapter';
2323
import { createSanitizedError } from '../Error';
2424

25+
// Query operators that always pass validation regardless of auth level.
26+
const queryOperators = ['$and', '$or', '$nor'];
27+
28+
// Registry of internal fields with access permissions.
29+
// Internal fields are never directly writable by clients, so clientWrite is omitted.
30+
// - clientRead: any client can use this field in queries
31+
// - masterRead: master key can use this field in queries
32+
// - masterWrite: master key can use this field in updates
33+
const internalFields = {
34+
_rperm: { clientRead: true, masterRead: true, masterWrite: true },
35+
_wperm: { clientRead: true, masterRead: true, masterWrite: true },
36+
_hashed_password: { clientRead: false, masterRead: false, masterWrite: true },
37+
_email_verify_token: { clientRead: false, masterRead: true, masterWrite: true },
38+
_perishable_token: { clientRead: false, masterRead: true, masterWrite: true },
39+
_perishable_token_expires_at: { clientRead: false, masterRead: true, masterWrite: true },
40+
_email_verify_token_expires_at: { clientRead: false, masterRead: true, masterWrite: true },
41+
_failed_login_count: { clientRead: false, masterRead: true, masterWrite: true },
42+
_account_lockout_expires_at: { clientRead: false, masterRead: true, masterWrite: true },
43+
_password_changed_at: { clientRead: false, masterRead: true, masterWrite: true },
44+
_password_history: { clientRead: false, masterRead: true, masterWrite: true },
45+
_tombstone: { clientRead: false, masterRead: true, masterWrite: false },
46+
_session_token: { clientRead: false, masterRead: true, masterWrite: false },
47+
/////////////////////////////////////////////////////////////////////////////////////////////
48+
// The following fields are not accessed by their _-prefixed name through the API;
49+
// they are mapped to REST-level names in the adapter layer or handled through
50+
// separate code paths.
51+
/////////////////////////////////////////////////////////////////////////////////////////////
52+
// System fields (mapped to REST-level names):
53+
// _id (objectId)
54+
// _created_at (createdAt)
55+
// _updated_at (updatedAt)
56+
// _last_used (lastUsed)
57+
// _expiresAt (expiresAt)
58+
/////////////////////////////////////////////////////////////////////////////////////////////
59+
// Legacy ACL format: mapped to/from _rperm/_wperm
60+
// _acl
61+
/////////////////////////////////////////////////////////////////////////////////////////////
62+
// Schema metadata: not data fields, used only for schema configuration
63+
// _metadata
64+
// _client_permissions
65+
/////////////////////////////////////////////////////////////////////////////////////////////
66+
// Dynamic auth data fields: used only in projections and updates, not in queries
67+
// _auth_data_<provider>
68+
};
69+
70+
// Derived access lists
71+
const specialQueryKeys = [
72+
...queryOperators,
73+
...Object.keys(internalFields).filter(k => internalFields[k].clientRead),
74+
];
75+
const specialMasterQueryKeys = [
76+
...queryOperators,
77+
...Object.keys(internalFields).filter(k => internalFields[k].masterRead),
78+
];
79+
2580
function addWriteACL(query, acl) {
2681
const newQuery = _.cloneDeep(query);
2782
//Can't be any existing '_wperm' query, we don't allow client queries on that, no need to $and
@@ -56,19 +111,6 @@ const transformObjectACL = ({ ACL, ...result }) => {
56111
return result;
57112
};
58113

59-
const specialQueryKeys = ['$and', '$or', '$nor', '_rperm', '_wperm'];
60-
const specialMasterQueryKeys = [
61-
...specialQueryKeys,
62-
'_email_verify_token',
63-
'_perishable_token',
64-
'_tombstone',
65-
'_email_verify_token_expires_at',
66-
'_failed_login_count',
67-
'_account_lockout_expires_at',
68-
'_password_changed_at',
69-
'_password_history',
70-
];
71-
72114
const validateQuery = (
73115
query: any,
74116
isMaster: boolean,
@@ -122,8 +164,8 @@ const validateQuery = (
122164
}
123165
if (
124166
!key.match(/^[a-zA-Z][a-zA-Z0-9_\.]*$/) &&
125-
((!specialQueryKeys.includes(key) && !isMaster && !update) ||
126-
(update && isMaster && !specialMasterQueryKeys.includes(key)))
167+
!specialQueryKeys.includes(key) &&
168+
!(isMaster && specialMasterQueryKeys.includes(key))
127169
) {
128170
throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid key name: ${key}`);
129171
}
@@ -250,17 +292,7 @@ const filterSensitiveData = (
250292
// acl: a list of strings. If the object to be updated has an ACL,
251293
// one of the provided strings must provide the caller with
252294
// write permissions.
253-
const specialKeysForUpdate = [
254-
'_hashed_password',
255-
'_perishable_token',
256-
'_email_verify_token',
257-
'_email_verify_token_expires_at',
258-
'_account_lockout_expires_at',
259-
'_failed_login_count',
260-
'_perishable_token_expires_at',
261-
'_password_changed_at',
262-
'_password_history',
263-
];
295+
const specialKeysForUpdate = Object.keys(internalFields).filter(k => internalFields[k].masterWrite);
264296

265297
const isSpecialUpdateKey = key => {
266298
return specialKeysForUpdate.indexOf(key) >= 0;

0 commit comments

Comments
 (0)
⚡