prevent prototype pollution via unsafe path

Author: dmlvr

packages/devextreme/js/__internal/core/m_errors.ts

@@ -64,6 +64,8 @@ export default errorUtils({
 
   E0122: 'AIIntegration: The sendRequest method is missing.',
 
+  E0123: 'Prototype pollution attempt detected: the path contains an unsafe fragment \'{0}\'',
+
   W0000: '\'{0}\' is deprecated in {1}. {2}',
 
   W0001: '{0} - \'{1}\' option is deprecated in {2}. {3}',

packages/devextreme/js/__internal/core/utils/m_data.ts

@@ -17,6 +17,10 @@ const bracketsToDots = function (expr) {
     .replace(/\]/g, '');
 };
 
+const UNSAFE_PATH_FRAGMENTS = new Set(['__proto__', 'constructor', 'prototype']);
+
+const isUnsafePathFragment = (name: string): boolean => UNSAFE_PATH_FRAGMENTS.has(name);
+
 export const getPathParts = function (name) {
   return bracketsToDots(name).split('.');
 };
@@ -81,6 +85,11 @@ export const compileGetter = function (expr) {
 
         const pathPart = path[i];
 
+        if (isUnsafePathFragment(pathPart)) {
+          errors.log('E0123', pathPart);
+          return;
+        }
+
         if (hasDefaultValue && isObject(current) && !(pathPart in current)) {
           return options.defaultValue;
         }
@@ -130,13 +139,23 @@ function combineGetters(getters) {
 
       for (let i = 0; i < last; i++) {
         const pathItem = path[i];
+        if (isUnsafePathFragment(pathItem)) {
+          errors.log('E0123', pathItem);
+          return;
+        }
         if (!(pathItem in current)) {
           current[pathItem] = { };
         }
         current = current[pathItem];
       }
 
-      current[path[last]] = value;
+      const lastPathItem = path[last];
+      if (isUnsafePathFragment(lastPathItem)) {
+        errors.log('E0123', lastPathItem);
+        return;
+      }
+
+      current[lastPathItem] = value;
     });
     return result;
   };
@@ -170,6 +189,11 @@ export const compileSetter = function (expr) {
     let currentValue = unwrap(obj, options);
 
     expr.forEach(function (propertyName, levelIndex) {
+      if (isUnsafePathFragment(propertyName)) {
+        errors.log('E0123', propertyName);
+        return;
+      }
+
       let propertyValue = readPropValue(currentValue, propertyName, options);
       const isPropertyFunc = !options.functionsAsIs && isFunction(propertyValue) && !isWrapped(propertyValue);
 

packages/devextreme/testing/tests/DevExpress.core/utils.data.tests.js

@@ -2,6 +2,7 @@ import {
     compileGetter as GETTER,
     compileSetter as SETTER
 } from 'core/utils/data';
+import errors from 'core/errors';
 import variableWrapper from 'core/utils/variable_wrapper';
 
 const mockVariableWrapper = {
@@ -480,6 +481,88 @@ QUnit.module('setter', () => {
 });
 
 
+QUnit.module('prototype pollution protection', {
+    beforeEach: function() {
+        sinon.spy(errors, 'log');
+    },
+    afterEach: function() {
+        errors.log.restore();
+        delete Object.prototype['pp_dx'];
+        delete Object.prototype['pp_dx2'];
+    }
+}, () => {
+
+    test('compileSetter logs error and skips assignment for __proto__ path fragment', function(assert) {
+        SETTER('__proto__.pp_dx')({}, 'yes', { functionsAsIs: true });
+
+        assert.strictEqual(errors.log.calledWith('E0123', '__proto__'), true, 'should log E0123 for __proto__');
+        assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
+    });
+
+    test('compileSetter logs error and skips assignment for constructor path fragment', function(assert) {
+        SETTER('constructor.prototype.pp_dx')({}, 'yes', { functionsAsIs: true });
+
+        assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor');
+        assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
+    });
+
+    test('compileSetter logs error and skips assignment for prototype path fragment', function(assert) {
+        const fn = function() {};
+        SETTER('prototype.pp_dx')(fn, 'yes', { functionsAsIs: true });
+
+        assert.strictEqual(errors.log.calledWith('E0123', 'prototype'), true, 'should log E0123 for prototype');
+        assert.strictEqual(fn.prototype.pp_dx, undefined, 'function prototype must not be modified');
+    });
+
+    test('compileSetter works normally for safe paths', function(assert) {
+        const obj = { a: { b: 1 } };
+        SETTER('a.b')(obj, 42);
+
+        assert.strictEqual(obj.a.b, 42, 'safe paths must still work');
+        assert.strictEqual(errors.log.called, false, 'should not log for safe paths');
+    });
+
+    test('compileGetter logs error and returns undefined for __proto__ path fragment', function(assert) {
+        const result = GETTER('__proto__.pp_dx')({});
+
+        assert.strictEqual(errors.log.calledWith('E0123', '__proto__'), true, 'should log E0123 for __proto__');
+        assert.strictEqual(result, undefined, 'getter must return undefined for __proto__');
+    });
+
+    test('compileGetter logs error and returns undefined for constructor path fragment', function(assert) {
+        const result = GETTER('constructor.prototype')(function() {});
+
+        assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor');
+        assert.strictEqual(result, undefined, 'getter must return undefined for constructor');
+    });
+
+    test('compileGetter logs error and returns undefined for prototype path fragment', function(assert) {
+        const result = GETTER('prototype.pp_dx')(function() {});
+
+        assert.strictEqual(errors.log.calledWith('E0123', 'prototype'), true, 'should log E0123 for prototype');
+        assert.strictEqual(result, undefined, 'getter must return undefined for prototype');
+    });
+
+    test('combineGetters logs error and skips __proto__ fragment, returns safe fields', function(assert) {
+        const obj = { safe: 'value' };
+        const result = GETTER(['__proto__.pp_dx', 'safe'])(obj);
+
+        assert.strictEqual(errors.log.calledWith('E0123', '__proto__'), true, 'should log E0123 for __proto__');
+        assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
+        assert.deepEqual(result, { safe: 'value' }, 'safe field must still be returned');
+    });
+
+    test('combineGetters logs error and skips constructor fragment, returns safe fields', function(assert) {
+        const obj = { safe: 'value' };
+        const result = GETTER(['constructor.prototype.pp_dx', 'safe'])(obj);
+
+        assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor');
+        assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
+        assert.deepEqual(result, { safe: 'value' }, 'safe field must still be returned');
+    });
+});
+
+
 QUnit.module('setter with wrapped variables', {
     beforeEach: function() {
         variableWrapper.inject(mockVariableWrapper);