[fix][core] keep segmentation values out of Object.prototype

.eslintrc.json

@@ -356,6 +356,13 @@
         ]
     },
     "overrides": [
+        {
+            "files": ["api/**/*.js", "plugins/*/api/**/*.js", "frontend/express/public/javascripts/countly/**/*.js", "plugins/*/frontend/**/*.js"],
+            "excludedFiles": ["**/tests.js", "**/tests/**"],
+            "rules": {
+                "no-prototype-pollution-sink": "error"
+            }
+        },
         {
             "files": [
                 "plugins/content/frontend/vite.config.js",

.github/workflows/main.yml

@@ -115,9 +115,12 @@ jobs:
 
       - name: ESLint
         shell: bash
+        # --rulesdir loads the in-repo rules in bin/eslint-rules. Grunt and lint-staged
+        # pass it too; without it here, eslint 8 fails every file matched by the
+        # no-prototype-pollution-sink override with "Definition for rule ... was not found".
         run: |
          npm install eslint@8.57.0 eslint-plugin-vue@9.31.0 @stylistic/eslint-plugin@2.11.0
-         npx eslint .
+         npx eslint . --rulesdir bin/eslint-rules
 
       - name: Check for any external web resources
         shell: bash

Gruntfile.js

@@ -3,7 +3,11 @@ module.exports = function(grunt) {
     grunt.initConfig({
         eslint: {
             options: {
-                configFile: './.eslintrc.json'
+                configFile: './.eslintrc.json',
+                //grunt-eslint forwards options to new ESLint(), and rulePaths is a valid
+                //eslint 8 option, so the local rules in bin/eslint-rules resolve by bare
+                //name without publishing a plugin. Keep in step with --rulesdir below.
+                rulePaths: ['./bin/eslint-rules']
             },
             target: ['./']
         },

api/api.js

@@ -532,6 +532,11 @@ function handleRequest(req, res) {
         form.parse(req, (err, fields, files) => {
             //handle bakcwards compatability with formiddble v1
             for (let i in files) {
+                // a key off a stored document or a parsed payload can be a prototype
+                // member name; writing through one would reach Object.prototype
+                if (i === "__proto__" || i === "constructor" || i === "prototype") {
+                    continue;
+                }
                 if (files[i].filepath) {
                     files[i].path = files[i].filepath;
                 }

api/jobs/topEvents.js

@@ -230,6 +230,11 @@ class TopEventsJob extends job.Job {
                 }
 
                 for (var event in data) {
+                    // a key off a stored document or a parsed payload can be a prototype
+                    // member name; writing through one would reach Object.prototype
+                    if (event === "__proto__" || event === "constructor" || event === "prototype") {
+                        continue;
+                    }
                     //Calculating trend
                     var trend = countlyCommon.getPercentChange(data[event].data.count["prev-total"], data[event].data.count.total);
                     data[event].data.count.change = trend.percent;

api/lib/countly.model.js

@@ -547,6 +547,11 @@ countlyModel.create = function(fetchValue) {
             return obj;
         }, periodObject);
         for (let i in data) {
+            // a key off a stored document or a parsed payload can be a prototype
+            // member name; writing through one would reach Object.prototype
+            if (i === "__proto__" || i === "constructor" || i === "prototype") {
+                continue;
+            }
             if (sparkLines[i]) {
                 data[i].sparkline = sparkLines[i].split(",").map(function(item) {
                     return parseInt(item);

api/parts/data/events.js

@@ -330,7 +330,7 @@ function processEvents(appEvents, appSegments, appSgValues, params, omitted_segm
                     continue;
                 }
                 //skip keys that map to object prototype members when used as field names
-                if (segKey === "__proto__" || segKey === "constructor" || segKey === "prototype") {
+                if (common.isForbiddenFieldName(segKey)) {
                     continue;
                 }
 
@@ -371,6 +371,15 @@ function processEvents(appEvents, appSegments, appSgValues, params, omitted_segm
                         tmpSegVal = "[CLY]" + tmpSegVal;
                     }
 
+                    //the value becomes a field name at d.<day>.<value>.<metric>; a value
+                    //naming an Object.prototype member is prefixed like the day numbers
+                    //above, so a later deepMerge of the stored document cannot walk it
+                    //into the prototype. The key is already guarded above; this is the
+                    //value path four lines down that the key guard does not reach.
+                    if (common.isForbiddenFieldName(tmpSegVal)) {
+                        tmpSegVal = "[CLY]" + tmpSegVal;
+                    }
+
                     tmpSegVal = common.encodeCharacters(tmpSegVal);
 
                     var postfix = common.crypto.createHash("md5").update(tmpSegVal).digest('base64')[0];
@@ -593,6 +602,14 @@ function mergeEvents(firstObj, secondObj) {
             continue;
         }
 
+        //a stored event document can carry "__proto__" as an OWN key, which the check
+        //above accepts. firstObj[firstLevel] is then Object.prototype, which is truthy,
+        //so the "if (!firstObj[firstLevel])" branch below does not fire either and the
+        //two writes land on the prototype for the life of the worker.
+        if (common.isForbiddenFieldName(firstLevel)) {
+            continue;
+        }
+
         if (!firstObj[firstLevel]) {
             firstObj[firstLevel] = secondObj[firstLevel];
             continue;

api/parts/data/exports.js

@@ -705,6 +705,11 @@ exports.fromRequest = function(options) {
                 }
                 if (options.columnNames || options.mapper) {
                     for (key in body) {
+                        // a key off a stored document or a parsed payload can be a prototype
+                        // member name; writing through one would reach Object.prototype
+                        if (key === "__proto__" || key === "constructor" || key === "prototype") {
+                            continue;
+                        }
                         if (options.mapper) {
                             body[key] = transformValuesInObject(body[key], options.mapper);
                         }

api/parts/data/fetch.js

@@ -214,6 +214,21 @@ fetch.fetchMergedEventData = function(params) {
     });
 };
 
+/**
+* Whether a key produced by for...in over a stored document may be walked into a
+* merge target. A field literally named __proto__ (or constructor / prototype)
+* deserializes as an own enumerable property, but indexing the target with it
+* resolves to the target's prototype rather than an own slot, so assigning through
+* it writes into Object.prototype for the life of the worker. Inherited keys are
+* skipped for the same reason.
+* @param {object} source - the object being iterated
+* @param {string} key - the key for...in produced
+* @returns {boolean} true when the key is safe to merge
+**/
+function isMergeableKey(source, key) {
+    return Object.prototype.hasOwnProperty.call(source, key) && !common.isForbiddenFieldName(key);
+}
+
 /**
 * Get merged data from multiple events in standard data model
 * @param {params} params - params object with app_id and date
@@ -247,6 +262,9 @@ fetch.getMergedEventData = function(params, events, options, callback) {
                 // delete allEventData[i].meta;
 
                 for (let levelOne in allEventData[i]) {
+                    if (!isMergeableKey(allEventData[i], levelOne)) {
+                        continue;
+                    }
                     if (typeof allEventData[i][levelOne] !== 'object') {
                         if (mergedEventOutput[levelOne]) {
                             mergedEventOutput[levelOne] += allEventData[i][levelOne];
@@ -257,6 +275,9 @@ fetch.getMergedEventData = function(params, events, options, callback) {
                     }
                     else {
                         for (let levelTwo in allEventData[i][levelOne]) {
+                            if (!isMergeableKey(allEventData[i][levelOne], levelTwo)) {
+                                continue;
+                            }
                             if (!mergedEventOutput[levelOne]) {
                                 mergedEventOutput[levelOne] = {};
                             }
@@ -271,6 +292,9 @@ fetch.getMergedEventData = function(params, events, options, callback) {
                             }
                             else {
                                 for (let levelThree in allEventData[i][levelOne][levelTwo]) {
+                                    if (!isMergeableKey(allEventData[i][levelOne][levelTwo], levelThree)) {
+                                        continue;
+                                    }
                                     if (!mergedEventOutput[levelOne][levelTwo]) {
                                         mergedEventOutput[levelOne][levelTwo] = {};
                                     }
@@ -285,6 +309,9 @@ fetch.getMergedEventData = function(params, events, options, callback) {
                                     }
                                     else {
                                         for (let levelFour in allEventData[i][levelOne][levelTwo][levelThree]) {
+                                            if (!isMergeableKey(allEventData[i][levelOne][levelTwo][levelThree], levelFour)) {
+                                                continue;
+                                            }
                                             if (!mergedEventOutput[levelOne][levelTwo][levelThree]) {
                                                 mergedEventOutput[levelOne][levelTwo][levelThree] = {};
                                             }
@@ -299,6 +326,9 @@ fetch.getMergedEventData = function(params, events, options, callback) {
                                             }
                                             else {
                                                 for (let levelFive in allEventData[i][levelOne][levelTwo][levelThree][levelFour]) {
+                                                    if (!isMergeableKey(allEventData[i][levelOne][levelTwo][levelThree][levelFour], levelFive)) {
+                                                        continue;
+                                                    }
                                                     if (!mergedEventOutput[levelOne][levelTwo][levelThree][levelFour]) {
                                                         mergedEventOutput[levelOne][levelTwo][levelThree][levelFour] = {};
                                                     }
@@ -322,6 +352,9 @@ fetch.getMergedEventData = function(params, events, options, callback) {
 
             meta = allEventData.map(x => x.meta).reduce((acc, x) => {
                 for (var key in x) {
+                    if (!isMergeableKey(x, key)) {
+                        continue;
+                    }
                     if (acc[key]) {
                         acc[key] = acc[key].concat(x[key]);
                     }
@@ -1813,6 +1846,12 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
     **/
     function deepMerge(ob1, ob2) {
         for (let i in ob2) {
+            //Skip anything that would let a stored field name reach a prototype. This
+            //is the durable guard: it also neutralises documents poisoned before the
+            //input paths were fixed, which re-pollute on every read otherwise.
+            if (!isMergeableKey(ob2, i)) {
+                continue;
+            }
             if (typeof ob1[i] === "undefined") {
                 ob1[i] = ob2[i];
             }
@@ -1881,6 +1920,9 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
                     //old meta merge
                     if (mergedDataObj.meta) {
                         for (let metaEl in dataObjects[i].meta) {
+                            if (!isMergeableKey(dataObjects[i].meta, metaEl)) {
+                                continue;
+                            }
                             if (mergedDataObj.meta[metaEl]) {
                                 mergedDataObj.meta[metaEl] = union(mergedDataObj.meta[metaEl], dataObjects[i].meta[metaEl]);
                             }
@@ -1896,6 +1938,9 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
                     //new meta merge as hash tables
                     if (dataObjects[i].meta_v2) {
                         for (let metaEl in dataObjects[i].meta_v2) {
+                            if (!isMergeableKey(dataObjects[i].meta_v2, metaEl)) {
+                                continue;
+                            }
                             if (mergedDataObj.meta[metaEl]) {
                                 mergedDataObj.meta[metaEl] = union(mergedDataObj.meta[metaEl], Object.keys(dataObjects[i].meta_v2[metaEl]));
                             }
@@ -1922,16 +1967,25 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
 
                     if (!isRefresh) {
                         for (let day in dataObjects[i].d) {
+                            if (!isMergeableKey(dataObjects[i].d, day)) {
+                                continue;
+                            }
                             if (options.unique.indexOf(day) !== -1) {
                                 continue;
                             }
                             for (let prop in dataObjects[i].d[day]) {
+                                if (!isMergeableKey(dataObjects[i].d[day], prop)) {
+                                    continue;
+                                }
                                 if (options.unique.indexOf(prop) !== -1 || prop <= 23 && prop >= 0) {
                                     continue;
                                 }
 
                                 if (typeof dataObjects[i].d[day][prop] === 'object') {
                                     for (let secondLevel in dataObjects[i].d[day][prop]) {
+                                        if (!isMergeableKey(dataObjects[i].d[day][prop], secondLevel)) {
+                                            continue;
+                                        }
                                         if ((levels.daily.length) ? levels.daily.indexOf(secondLevel) !== -1 : options.unique.indexOf(secondLevel) === -1) {
                                             if (!mergedDataObj[year][month][prop]) {
                                                 mergedDataObj[year][month][prop] = {};
@@ -1980,6 +2034,9 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
             }
             //Fixing meta  to be escaped.(Because return output will escape keys and make values incompatable)
             for (let i in mergedDataObj.meta) {
+                if (!isMergeableKey(mergedDataObj.meta, i)) {
+                    continue;
+                }
                 for (var p = 0; p < mergedDataObj.meta[i].length; p++) {
                     if (mergedDataObj.meta[i][p] && typeof mergedDataObj.meta[i][p] === 'string') {
                         mergedDataObj.meta[i][p] = mergedDataObj.meta[i][p].replace(new RegExp("\"", "g"), '&quot;');
@@ -1991,6 +2048,9 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
                 var metric_length = plugins.getConfig("api", params.app && params.app.plugins, true).metric_limit;
                 if (metric_length > 0) {
                     for (let i in mergedDataObj.meta) {
+                        if (!isMergeableKey(mergedDataObj.meta, i)) {
+                            continue;
+                        }
                         if (mergedDataObj.meta[i].length > metric_length) {
                             delete mergedDataObj.meta[i]; //don't  return if there is more than limit
                         }
@@ -2002,6 +2062,9 @@ function fetchTimeObj(collection, params, isCustomEvent, options, callback) {
                     var value_length = plugins.getConfig("api", params.app && params.app.plugins, true).event_segmentation_value_limit;
                     if (value_length > 0) {
                         for (let i in mergedDataObj.meta) {
+                            if (!isMergeableKey(mergedDataObj.meta, i)) {
+                                continue;
+                            }
                             if (mergedDataObj.meta[i].length > value_length) {
                                 mergedDataObj.meta[i].splice(value_length); //removes some elements if there is more than set limit
                             }

api/parts/mgmt/app_users.js

@@ -490,6 +490,11 @@ usersApi.mergeOtherPlugins = function(options, callback) {
 
 usersApi.mergeUserProperties = function(newAppUserP, oldAppUser) {
     for (var i in oldAppUser) {
+        // a key off a stored document or a parsed payload can be a prototype
+        // member name; writing through one would reach Object.prototype
+        if (i === "__proto__" || i === "constructor" || i === "prototype") {
+            continue;
+        }
         // sum up session count and total session duration
         if (i === "sc" || i === "tsd") {
             if (typeof newAppUserP[i] === "undefined") {