From d245dc278ea8313b3d09c3b85f21074860f90f9f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 22:46:31 +0300 Subject: [PATCH 1/2] security(compliance-hub): fix the consents table projection server side /o/app_users/consents built its projection as `params.qstring.project || params.qstring.projection || {defaults}`, so the narrow default applied only when the caller omitted both parameters. Because a query-string value arrives as a string, `project={}` is truthy and short-circuits the chain, and an empty projection means "all fields" to MongoDB. The projection was then forwarded unvalidated into appUsers.search() and on to find(query, project). A member holding nothing but compliance_hub read on an app could therefore turn a consent-status table into full app_users documents: name, email, username, phone, organization, customer-defined custom properties, payment totals, city/region, acquisition source and device details. The Compliance Hub UI never sends a projection, so the parameter was attacker-facing only. The consent table renders a fixed set of columns, so the projection is now a server-side constant and any project/projection on the request is ignored. The allowlist matches the columns bound in the users table template, plus uid and appUserExport for the per-row history and export actions. /o/consent/search is fixed the same way. Its response shape is unchanged - full consent_history documents, which hold only consent state and the device context it changed in - but the projection is no longer taken from the request, and the now-unreachable JSON.parse of the caller's value is removed. Note the surrounding hardening pass validated user-supplied *queries* via common.parseUserQuery but never covered projections; the only projection sanitizer in the tree (dbviewer's sanitizeProjection) constrains values to 0/1 and not which fields may be requested, so it would not have closed this. Regression tests assert that project={}, an explicit projection naming name/email/username/custom, and the projection alias parameter all return only the allowed fields. Co-Authored-By: Claude --- CHANGELOG.md | 3 ++ plugins/compliance-hub/api/api.js | 46 ++++++++++++++------ plugins/compliance-hub/tests.js | 71 +++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 963b1750c10..79b1c1ea48d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ ## Version 24.05.51 +Security Fixes: +- [compliance-hub] The consents table now returns a fixed set of fields; a projection supplied on the request is no longer used to widen the response beyond the consent columns + Enterprise Fixes: - [data-manager] Fixed bug where event and view transformations occasionally failed to apply to incoming data diff --git a/plugins/compliance-hub/api/api.js b/plugins/compliance-hub/api/api.js index d983c89c006..1e0f036a86b 100644 --- a/plugins/compliance-hub/api/api.js +++ b/plugins/compliance-hub/api/api.js @@ -10,6 +10,25 @@ var plugin = {}, const FEATURE_NAME = 'compliance_hub'; +/** + * Fields the Compliance Hub users table renders, and the only app_users fields + * /o/app_users/consents may return. These match the columns bound in + * frontend/public/templates/user.html: did, d, av, consent, lac, plus uid and + * appUserExport for the per-row consent-history and export actions. + * + * compliance_hub read is a consent-scoped permission, so this endpoint must not + * be able to return the rest of an app_users document. + */ +const CONSENT_TABLE_PROJECTION = Object.freeze({ + "did": 1, + "d": 1, + "av": 1, + "consent": 1, + "lac": 1, + "uid": 1, + "appUserExport": 1 +}); + (function() { plugins.register("/permissions/features", function(ob) { ob.features.push(FEATURE_NAME); @@ -177,7 +196,12 @@ const FEATURE_NAME = 'compliance_hub'; } else if (total > 0) { params.qstring.query = params.qstring.query || params.qstring.filter || {}; - params.qstring.project = params.qstring.project || params.qstring.projection || {}; + //consent_history holds only consent state and the device + //context it changed in, so full documents stay the response + //shape here. The projection is still fixed server side + //rather than taken from the request, so this endpoint + //cannot be steered with project/projection either. + params.qstring.project = {}; var parsedSearch = common.parseUserQuery(params.qstring.query); if (parsedSearch.error) { @@ -209,16 +233,6 @@ const FEATURE_NAME = 'compliance_hub'; params.qstring.query.ts = countlyCommon.getTimestampRangeQuery(params, false); } - params.qstring.project = params.qstring.project || {}; - if (typeof params.qstring.project === "string" && params.qstring.project.length) { - try { - params.qstring.project = JSON.parse(params.qstring.project); - } - catch (ex) { - params.qstring.project = {}; - } - } - params.qstring.sort = params.qstring.sort || {}; if (typeof params.qstring.sort === "string" && params.qstring.sort.length) { try { @@ -294,7 +308,15 @@ const FEATURE_NAME = 'compliance_hub'; return; } params.qstring.query = parsedC.query; - params.qstring.project = params.qstring.project || params.qstring.projection || {"did": 1, "d": 1, "av": 1, "consent": 1, "lac": 1, "uid": 1, "appUserExport": 1}; + //The consent table is a fixed set of columns, so the + //projection is fixed server side and any project/projection + //supplied by the caller is ignored. Previously the default + //applied only when the caller omitted both parameters, so + //project={} - an empty projection, which Mongo reads as "all + //fields" - returned whole app_users documents (name, email, + //phone, custom properties, payment totals, location) to a + //member holding nothing but compliance_hub read. + params.qstring.project = Object.assign({}, CONSENT_TABLE_PROJECTION); if (params.qstring.sSearch && params.qstring.sSearch !== "") { params.qstring.query.did = {"$regex": new RegExp(".*" + params.qstring.sSearch + ".*", 'i')}; diff --git a/plugins/compliance-hub/tests.js b/plugins/compliance-hub/tests.js index af58c85f6ee..59b81accb4b 100644 --- a/plugins/compliance-hub/tests.js +++ b/plugins/compliance-hub/tests.js @@ -83,6 +83,14 @@ describe('Testing Compliance Hub', function() { }); }); describe('Check app_users consents', function() { + // The consent table is a fixed set of columns. The projection used to be + // taken from the request when supplied, so project={} - which Mongo reads + // as "all fields" - returned whole app_users documents (name, email, + // phone, custom properties, payment totals, location) to any caller with + // compliance_hub read. The projection is now fixed server side. + // _id is included because Mongo returns it unless explicitly excluded. + var ALLOWED_FIELDS = ["_id", "did", "d", "av", "consent", "lac", "uid", "appUserExport"]; + it('should list app users with consent data', function(done) { request .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID) @@ -96,6 +104,69 @@ describe('Testing Compliance Hub', function() { setTimeout(done, 100); }); }); + + it('should ignore an empty projection supplied by the caller', function(done) { + request + .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&project=' + encodeURIComponent('{}')) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('aaData'); + ob.aaData.should.be.an.Array; + ob.aaData.forEach(function(row) { + Object.keys(row).forEach(function(field) { + ALLOWED_FIELDS.should.containEql(field); + }); + }); + setTimeout(done, 100); + }); + }); + + it('should ignore an explicit projection asking for extra fields', function(done) { + var project = JSON.stringify({uid: 1, did: 1, name: 1, email: 1, username: 1, custom: 1, consent: 1}); + request + .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&project=' + encodeURIComponent(project)) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('aaData'); + ob.aaData.forEach(function(row) { + row.should.not.have.property('name'); + row.should.not.have.property('email'); + row.should.not.have.property('username'); + row.should.not.have.property('custom'); + Object.keys(row).forEach(function(field) { + ALLOWED_FIELDS.should.containEql(field); + }); + }); + setTimeout(done, 100); + }); + }); + + it('should ignore the projection alias parameter as well', function(done) { + request + .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&projection=' + encodeURIComponent('{}')) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('aaData'); + ob.aaData.forEach(function(row) { + Object.keys(row).forEach(function(field) { + ALLOWED_FIELDS.should.containEql(field); + }); + }); + setTimeout(done, 100); + }); + }); }); describe('Reset App', function() { From a2e4905addeea9e067ccbfa9a392de55dfbc9a2b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:04:42 +0300 Subject: [PATCH 2/2] fix(compliance-hub): leave the consent search projections as they were Addresses review feedback on the countly-platform port. The consent-search endpoints were changed to a fixed empty projection for tidiness, which was scope creep: their default was already an empty projection, so a caller could only ever narrow the response and there was no vulnerability there to fix. Forcing {} removed the ability to narrow. Restored to their original behaviour, including the JSON.parse of a caller-supplied value. The only change left is the one that fixes the actual issue: /o/app_users/consents returns a fixed set of fields. Co-Authored-By: Claude --- plugins/compliance-hub/api/api.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/plugins/compliance-hub/api/api.js b/plugins/compliance-hub/api/api.js index 1e0f036a86b..3259c6e45b9 100644 --- a/plugins/compliance-hub/api/api.js +++ b/plugins/compliance-hub/api/api.js @@ -196,12 +196,7 @@ const CONSENT_TABLE_PROJECTION = Object.freeze({ } else if (total > 0) { params.qstring.query = params.qstring.query || params.qstring.filter || {}; - //consent_history holds only consent state and the device - //context it changed in, so full documents stay the response - //shape here. The projection is still fixed server side - //rather than taken from the request, so this endpoint - //cannot be steered with project/projection either. - params.qstring.project = {}; + params.qstring.project = params.qstring.project || params.qstring.projection || {}; var parsedSearch = common.parseUserQuery(params.qstring.query); if (parsedSearch.error) { @@ -233,6 +228,16 @@ const CONSENT_TABLE_PROJECTION = Object.freeze({ params.qstring.query.ts = countlyCommon.getTimestampRangeQuery(params, false); } + params.qstring.project = params.qstring.project || {}; + if (typeof params.qstring.project === "string" && params.qstring.project.length) { + try { + params.qstring.project = JSON.parse(params.qstring.project); + } + catch (ex) { + params.qstring.project = {}; + } + } + params.qstring.sort = params.qstring.sort || {}; if (typeof params.qstring.sort === "string" && params.qstring.sort.length) { try {