From c97b2001c61170dfd598561c10f5f4c6ad27e451 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 4d91cd35b8e..b775c251472 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Fixes: - [events] Fix sum chart tooltip displaying the raw floating-point value instead of a rounded number +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 + ## Version 25.03.50 Fixes: - [star-rating] Fix custom widget logo resolving to the wrong path (mis-detected as the global app logo) after editing a widget 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 7bf65959130f77c78d122bb6830bfd783362777d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:04:14 +0300 Subject: [PATCH 2/2] fix(compliance-hub): leave the consent search projections as they were Addresses review feedback on the countly-platform port of this change. The consent-search endpoints were changed to a fixed empty projection for tidiness, to remove the caller-supplied-projection pattern from the plugin. That was scope creep, and in one respect it made things worse. Their default was already an empty projection, so a caller could only ever narrow the response, never widen it, and there was no vulnerability there to fix. Forcing {} removed the ability to narrow. On countly-platform the same endpoint reads drill_events and flattens a user-property snapshot into every row, so a caller that had been requesting a narrow set of fields would have started receiving user properties. This branch reads consent_history, which holds no such fields, so the effect here is only the lost ability to minimise, but the two repositories should behave the same. Both search endpoints are 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 {