Skip to content

Commit be1d65d

Browse files
authored
fix: Protected fields bypass via logical query operators ([GHSA-72hp-qff8-4pvv](GHSA-72hp-qff8-4pvv)) (#10140)
1 parent e9b020b commit be1d65d

File tree

2 files changed

+156
-8
lines changed

2 files changed

+156
-8
lines changed

spec/ProtectedFields.spec.js

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1700,4 +1700,141 @@ describe('ProtectedFields', function () {
17001700
done();
17011701
});
17021702
});
1703+
1704+
describe('query on protected fields via logical operators', function () {
1705+
let user;
1706+
let otherUser;
1707+
const testEmail = 'victim@example.com';
1708+
const otherEmail = 'other@example.com';
1709+
1710+
beforeEach(async function () {
1711+
await reconfigureServer({
1712+
protectedFields: {
1713+
_User: { '*': ['email'] },
1714+
},
1715+
});
1716+
user = new Parse.User();
1717+
user.setUsername('victim' + Date.now());
1718+
user.setPassword('password');
1719+
user.setEmail(testEmail);
1720+
const acl = new Parse.ACL();
1721+
acl.setPublicReadAccess(true);
1722+
user.setACL(acl);
1723+
await user.save(null, { useMasterKey: true });
1724+
1725+
otherUser = new Parse.User();
1726+
otherUser.setUsername('attacker' + Date.now());
1727+
otherUser.setPassword('password');
1728+
otherUser.setEmail(otherEmail);
1729+
const acl2 = new Parse.ACL();
1730+
acl2.setPublicReadAccess(true);
1731+
otherUser.setACL(acl2);
1732+
await otherUser.save(null, { useMasterKey: true });
1733+
await Parse.User.logIn(otherUser.getUsername(), 'password');
1734+
});
1735+
1736+
it('should deny query on protected field via $or', async function () {
1737+
const q1 = new Parse.Query(Parse.User);
1738+
q1.equalTo('email', testEmail);
1739+
const query = Parse.Query.or(q1);
1740+
await expectAsync(query.find()).toBeRejectedWith(
1741+
jasmine.objectContaining({
1742+
code: Parse.Error.OPERATION_FORBIDDEN,
1743+
})
1744+
);
1745+
});
1746+
1747+
it('should deny query on protected field via $and', async function () {
1748+
const query = new Parse.Query(Parse.User);
1749+
query.withJSON({ where: { $and: [{ email: testEmail }] } });
1750+
await expectAsync(query.find()).toBeRejectedWith(
1751+
jasmine.objectContaining({
1752+
code: Parse.Error.OPERATION_FORBIDDEN,
1753+
})
1754+
);
1755+
});
1756+
1757+
it('should deny query on protected field via $nor', async function () {
1758+
const query = new Parse.Query(Parse.User);
1759+
query.withJSON({ where: { $nor: [{ email: testEmail }] } });
1760+
await expectAsync(query.find()).toBeRejectedWith(
1761+
jasmine.objectContaining({
1762+
code: Parse.Error.OPERATION_FORBIDDEN,
1763+
})
1764+
);
1765+
});
1766+
1767+
it('should deny query on protected field via nested $or inside $and', async function () {
1768+
const query = new Parse.Query(Parse.User);
1769+
query.withJSON({ where: { $and: [{ $or: [{ email: testEmail }] }] } });
1770+
await expectAsync(query.find()).toBeRejectedWith(
1771+
jasmine.objectContaining({
1772+
code: Parse.Error.OPERATION_FORBIDDEN,
1773+
})
1774+
);
1775+
});
1776+
1777+
it('should deny query on protected field via $or with $regex', async function () {
1778+
const query = new Parse.Query(Parse.User);
1779+
query.withJSON({ where: { $or: [{ email: { $regex: '^victim' } }] } });
1780+
await expectAsync(query.find()).toBeRejectedWith(
1781+
jasmine.objectContaining({
1782+
code: Parse.Error.OPERATION_FORBIDDEN,
1783+
})
1784+
);
1785+
});
1786+
1787+
it('should allow $or query on non-protected fields', async function () {
1788+
const q1 = new Parse.Query(Parse.User);
1789+
q1.equalTo('username', user.getUsername());
1790+
const query = Parse.Query.or(q1);
1791+
const results = await query.find();
1792+
expect(results.length).toBe(1);
1793+
expect(results[0].id).toBe(user.id);
1794+
});
1795+
1796+
it('should allow master key to query on protected fields via $or', async function () {
1797+
const q1 = new Parse.Query(Parse.User);
1798+
q1.equalTo('email', testEmail);
1799+
const query = Parse.Query.or(q1);
1800+
const results = await query.find({ useMasterKey: true });
1801+
expect(results.length).toBe(1);
1802+
expect(results[0].id).toBe(user.id);
1803+
});
1804+
1805+
it('should deny query on protected field with falsy value', async function () {
1806+
const query = new Parse.Query(Parse.User);
1807+
query.withJSON({ where: { email: null } });
1808+
await expectAsync(query.find()).toBeRejectedWith(
1809+
jasmine.objectContaining({
1810+
code: Parse.Error.OPERATION_FORBIDDEN,
1811+
})
1812+
);
1813+
});
1814+
1815+
it('should deny query on protected field with falsy value via $or', async function () {
1816+
const query = new Parse.Query(Parse.User);
1817+
query.withJSON({ where: { $or: [{ email: null }] } });
1818+
await expectAsync(query.find()).toBeRejectedWith(
1819+
jasmine.objectContaining({
1820+
code: Parse.Error.OPERATION_FORBIDDEN,
1821+
})
1822+
);
1823+
});
1824+
1825+
it('should not throw TypeError in denyProtectedFields for null element in $or', async function () {
1826+
const Config = require('../lib/Config');
1827+
const authModule = require('../lib/Auth');
1828+
const RestQuery = require('../lib/RestQuery');
1829+
const config = Config.get(Parse.applicationId);
1830+
const restQuery = await RestQuery({
1831+
method: RestQuery.Method.find,
1832+
config,
1833+
auth: authModule.nobody(config),
1834+
className: '_User',
1835+
restWhere: { $or: [null, { username: 'test' }] },
1836+
});
1837+
await expectAsync(restQuery.denyProtectedFields()).toBeResolved();
1838+
});
1839+
});
17031840
});

src/RestQuery.js

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -847,15 +847,26 @@ _UnsafeRestQuery.prototype.denyProtectedFields = async function () {
847847
this.auth,
848848
this.findOptions
849849
) || [];
850-
for (const key of protectedFields) {
851-
if (this.restWhere[key]) {
852-
throw createSanitizedError(
853-
Parse.Error.OPERATION_FORBIDDEN,
854-
`This user is not allowed to query ${key} on class ${this.className}`,
855-
this.config
856-
);
850+
const checkWhere = (where) => {
851+
if (typeof where !== 'object' || where === null) {
852+
return;
857853
}
858-
}
854+
for (const key of protectedFields) {
855+
if (key in where) {
856+
throw createSanitizedError(
857+
Parse.Error.OPERATION_FORBIDDEN,
858+
`This user is not allowed to query ${key} on class ${this.className}`,
859+
this.config
860+
);
861+
}
862+
}
863+
for (const op of ['$or', '$and', '$nor']) {
864+
if (Array.isArray(where[op])) {
865+
where[op].forEach(subQuery => checkWhere(subQuery));
866+
}
867+
}
868+
};
869+
checkWhere(this.restWhere);
859870
};
860871

861872
// Augments this.response with all pointers on an object

0 commit comments

Comments
 (0)