From 074e9cd410efba4c881ae5f7beb6bc9d78757038 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 17:19:54 +0300 Subject: [PATCH] security(star-rating): authorize the /o?method=star ratings read Backport of #7955 to release.24.05. The star-rating dashboard read performed no authorization. Authentication on /o is per method: core methods call validateUserForDataReadAPI themselves, and the default branch hands the validators to plugins as helpers without calling them, so a plugin that claims a request must authorize it. This branch claimed the request and never did, so it answered callers with no account, token or session, for any app_id. Wrap it in validateRead(params, FEATURE_NAME, ...), the same check the sibling reads in this file already apply, running before the period parameter is validated so an unauthorized caller cannot probe the endpoint. The only caller is the dashboard Ratings page, which sends the session credential, so it is unaffected. Reported through the security bug bounty programme (received 2026-08-18). Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 3 + plugins/star-rating/api/api.js | 130 +++++++++++++++++---------------- 2 files changed, 71 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7703fab7885..5948e3b6798 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ Fixes: Enterprise Fixes: - [data-manager] Fixed editing an event whose key contains `&` creating undeletable duplicate rows in the events table +Security Fixes: +- [star-rating] The `/o?method=star` ratings read now requires star-rating read access to the application it is asked about. It previously performed no authorization, so the platform and application-version combinations that had received ratings could be read for any application by a caller with no account, token or session + ## Version 24.05.51 Fixes: diff --git a/plugins/star-rating/api/api.js b/plugins/star-rating/api/api.js index 4ca7d12cc21..068d83d5744 100644 --- a/plugins/star-rating/api/api.js +++ b/plugins/star-rating/api/api.js @@ -1568,78 +1568,84 @@ function uploadFile(myfile, id, callback) { plugins.register('/o', function(ob) { var params = ob.params; if (params.qstring.method === 'star') { - if (params.qstring.period) { - //check if period comes from datapicker - if (params.qstring.period.indexOf(",") !== -1) { - try { - params.qstring.period = JSON.parse(params.qstring.period); - } - catch (SyntaxError) { - common.returnMessage(params, 400, 'Bad request parameter: period'); - return true; - } - } - else { - switch (params.qstring.period) { - case "prevMonth": - case "month": - case "day": - case "yesterday": - case "hour": - break; - default: - if (!/([0-9]+)days/.test(params.qstring.period)) { + //this read is app scoped: require the caller to hold star-rating + //read access on app_id, the same check the sibling reads in this + //file apply. Authorize before validating parameters so that an + //unauthorized caller cannot probe the endpoint. + validateRead(params, FEATURE_NAME, function() { + if (params.qstring.period) { + //check if period comes from datapicker + if (params.qstring.period.indexOf(",") !== -1) { + try { + params.qstring.period = JSON.parse(params.qstring.period); + } + catch (SyntaxError) { common.returnMessage(params, 400, 'Bad request parameter: period'); return true; } - break; + } + else { + switch (params.qstring.period) { + case "prevMonth": + case "month": + case "day": + case "yesterday": + case "hour": + break; + default: + if (!/([0-9]+)days/.test(params.qstring.period)) { + common.returnMessage(params, 400, 'Bad request parameter: period'); + return true; + } + break; + } } } - } - else { - common.returnMessage(params, 400, 'Missing request parameter: period'); - return true; - } - countlyCommon.setPeriod(params.qstring.period, true); - var periodObj = countlyCommon.periodObj; - var collectionName = 'events' + crypto.createHash('sha1').update('[CLY]_star_rating' + params.qstring.app_id).digest('hex'); - var documents = []; - for (var i = 0; i < periodObj.reqZeroDbDateIds.length; i++) { - documents.push("no-segment_" + periodObj.reqZeroDbDateIds[i]); - for (var m = 0; m < common.base64.length; m++) { - documents.push("no-segment_" + periodObj.reqZeroDbDateIds[i] + "_" + common.base64[m]); + else { + common.returnMessage(params, 400, 'Missing request parameter: period'); + return true; } - } - common.db.collection(collectionName).find({ - '_id': { - $in: documents + countlyCommon.setPeriod(params.qstring.period, true); + var periodObj = countlyCommon.periodObj; + var collectionName = 'events' + crypto.createHash('sha1').update('[CLY]_star_rating' + params.qstring.app_id).digest('hex'); + var documents = []; + for (var i = 0; i < periodObj.reqZeroDbDateIds.length; i++) { + documents.push("no-segment_" + periodObj.reqZeroDbDateIds[i]); + for (var m = 0; m < common.base64.length; m++) { + documents.push("no-segment_" + periodObj.reqZeroDbDateIds[i] + "_" + common.base64[m]); + } } - }).toArray(function(err, docs) { - if (!err) { - var result = {}; - docs.forEach(function(doc) { - if (!doc.meta) { - doc.meta = {}; - } - if (!doc.meta.platform_version_rate) { - doc.meta.platform_version_rate = []; - } - if (doc.meta_v2 && doc.meta_v2.platform_version_rate) { - common.arrayAddUniq(doc.meta.platform_version_rate, Object.keys(doc.meta_v2.platform_version_rate)); - } - doc.meta.platform_version_rate.forEach(function(item) { - var data = item.split('**'); - if (result[data[0]] === undefined) { - result[data[0]] = []; + common.db.collection(collectionName).find({ + '_id': { + $in: documents + } + }).toArray(function(err, docs) { + if (!err) { + var result = {}; + docs.forEach(function(doc) { + if (!doc.meta) { + doc.meta = {}; + } + if (!doc.meta.platform_version_rate) { + doc.meta.platform_version_rate = []; } - if (result[data[0]].indexOf(data[1]) === -1) { - result[data[0]].push(data[1]); + if (doc.meta_v2 && doc.meta_v2.platform_version_rate) { + common.arrayAddUniq(doc.meta.platform_version_rate, Object.keys(doc.meta_v2.platform_version_rate)); } + doc.meta.platform_version_rate.forEach(function(item) { + var data = item.split('**'); + if (result[data[0]] === undefined) { + result[data[0]] = []; + } + if (result[data[0]].indexOf(data[1]) === -1) { + result[data[0]].push(data[1]); + } + }); }); - }); - common.returnOutput(params, result); - return true; - } + common.returnOutput(params, result); + return true; + } + }); }); return true; }