Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/devextreme/js/__internal/core/m_errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ export default errorUtils({

E0122: 'AIIntegration: The sendRequest method is missing.',

E0123: '{0} received an unsafe expression: \'{1}\'',

W0000: '\'{0}\' is deprecated in {1}. {2}',

W0001: '{0} - \'{1}\' option is deprecated in {2}. {3}',
Expand Down
23 changes: 23 additions & 0 deletions packages/devextreme/js/__internal/core/utils/m_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('.');
};
Expand Down Expand Up @@ -64,6 +68,15 @@ export const compileGetter = function (expr) {

if (typeof expr === 'string') {
const path = getPathParts(expr);
const unsafeFragment = path.find(isUnsafePathFragment);

if (unsafeFragment !== undefined) {
return function () {
errors.log('E0123', 'compileGetter', unsafeFragment);
// eslint-disable-next-line no-useless-return
return;
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • return function (obj, options) {
  •  if (unsafeFragment !== undefined) {
    
  • if (unsafeFragment !== undefined) {
  •  return function () {
       errors.log('E0123', unsafeFragment);
    
  •    return;
    
  •  }
    
  •  };
    
  • }

  • return function (obj, options) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

return function (obj, options) {
options = prepareOptions(options);
Expand Down Expand Up @@ -164,9 +177,19 @@ const ensurePropValueDefined = function (obj, propName, value, options) {
export const compileSetter = function (expr) {
expr = getPathParts(expr || 'this');
const lastLevelIndex = expr.length - 1;
const unsafeFragment = expr.find(isUnsafePathFragment);

if (unsafeFragment !== undefined) {
return function () {
errors.log('E0123', 'compileSetter', unsafeFragment);
// eslint-disable-next-line no-useless-return
return;
};
}

return function (obj, value, options) {
options = prepareOptions(options);

let currentValue = unwrap(obj, options);

expr.forEach(function (propertyName, levelIndex) {
Expand Down
114 changes: 114 additions & 0 deletions packages/devextreme/testing/tests/DevExpress.core/utils.data.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -480,6 +481,119 @@ 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 (T1330839)', function(assert) {
const obj = {};
SETTER('__proto__.pp_dx')(obj, 'yes', { functionsAsIs: true });

assert.strictEqual(errors.log.calledWith('E0123', 'compileSetter', '__proto__'), true, 'should log E0123 for __proto__');
assert.strictEqual(obj.pp_dx, undefined, 'target object must not be modified');
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
});

test('compileSetter logs error and skips assignment for constructor path fragment (T1330839)', function(assert) {
const obj = {};
SETTER('constructor.prototype.pp_dx')(obj, 'yes', { functionsAsIs: true });

assert.strictEqual(errors.log.calledWith('E0123', 'compileSetter', 'constructor'), true, 'should log E0123 for constructor');
assert.strictEqual(obj.pp_dx, undefined, 'target object must not be modified');
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
});

test('compileSetter logs error and skips assignment for prototype path fragment (T1330839)', function(assert) {
const fn = function() {};
SETTER('prototype.pp_dx')(fn, 'yes', { functionsAsIs: true });

assert.strictEqual(errors.log.calledWith('E0123', 'compileSetter', 'prototype'), true, 'should log E0123 for prototype');
assert.strictEqual(fn.pp_dx, undefined, 'function object must not be modified');
assert.strictEqual(fn.prototype.pp_dx, undefined, 'function prototype must not be modified');
});

test('compileSetter works normally for safe paths (T1330839)', 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('compileSetter blocks unsafe fragment written in bracket notation (T1330839)', function(assert) {
const obj = {};
SETTER('a[constructor][prototype].pp_dx')(obj, 'yes', { functionsAsIs: true });

assert.strictEqual(errors.log.calledWith('E0123', 'compileSetter', 'constructor'), true, 'should log E0123 for constructor in bracket notation');
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
});

test('compileSetter blocks unsafe fragment in non-first position (T1330839)', function(assert) {
const obj = { a: { b: {} } };
SETTER('a.b.__proto__')(obj, { pp_dx: 'yes' }, { functionsAsIs: true });

assert.strictEqual(errors.log.calledWith('E0123', 'compileSetter', '__proto__'), true, 'should log E0123 for __proto__ in non-first position');
assert.strictEqual(obj.a.b.pp_dx, undefined, 'nested object must not inherit a polluted property');
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
});

test('compileGetter logs error and returns undefined for __proto__ path fragent (T1330839)', function(assert) {
const result = GETTER('__proto__.pp_dx')({});

assert.strictEqual(errors.log.calledWith('E0123', 'compileGetter', '__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 (T1330839)', function(assert) {
const result = GETTER('constructor.prototype')(function() {});

assert.strictEqual(errors.log.calledWith('E0123', 'compileGetter', '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 (T1330839)', function(assert) {
const result = GETTER('prototype.pp_dx')(function() {});

assert.strictEqual(errors.log.calledWith('E0123', 'compileGetter', '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 (T1330839)', function(assert) {
const obj = { safe: 'value' };
const result = GETTER(['__proto__.pp_dx', 'safe'])(obj);

assert.strictEqual(errors.log.calledWith('E0123', 'compileGetter', '__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 (T1330839)', function(assert) {
const obj = { safe: 'value' };
const result = GETTER(['constructor.prototype.pp_dx', 'safe'])(obj);

assert.strictEqual(errors.log.calledWith('E0123', 'compileGetter', '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');
});

test('combineGetters logs error and skips unsafe paths even if intermediate values break early (with defaultValue) (T1330839)', function(assert) {
const obj = { a: null, safe: 'value' };
const result = GETTER(['a.constructor.prototype.pp_dx2', 'safe'])(obj, { defaultValue: 'default' });

assert.strictEqual(errors.log.calledWith('E0123', 'compileGetter', 'constructor'), true, 'should log E0123 for constructor');
assert.strictEqual(({}).pp_dx2, undefined, 'Object.prototype must not be polluted');
assert.deepEqual(result, { safe: 'value' }, 'safe field must still be returned');
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test('compileSetter blocks unsafe fragment written in bracket notation (T1330839)', function(assert) {
    const obj = {};
    SETTER('a[constructor][prototype].pp_dx')(obj, 'yes', { functionsAsIs: true });

    assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor in bracket notation');
    assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
});

test('compileSetter blocks unsafe fragment in non-first position (T1330839)', function(assert) {
    const obj = { a: { b: {} } };
    SETTER('a.b.__proto__')(obj, { pp_dx: 'yes' }, { functionsAsIs: true });

    assert.strictEqual(errors.log.calledWith('E0123', '__proto__'), true, 'should log E0123 for __proto__ in non-first position');
    assert.strictEqual(obj.a.b.pp_dx, undefined, 'nested object must not inherit a polluted property');
    assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


QUnit.module('setter with wrapped variables', {
beforeEach: function() {
variableWrapper.inject(mockVariableWrapper);
Expand Down
Loading