Skip to content

Commit

Permalink
fix(security): regexp (#4924)
Browse files Browse the repository at this point in the history
* fix(cmf/security): regexp

* fix: regexp security issue

* chore: secu

* Update settings.js

* Update json-refs file path

* Fix polynomial regular expression issue by using trim

* fix: withoutHOC regex by removing unused nonMemoized function

* Escape regexp to fix tests

---------

Co-authored-by: Alexandre Amalric <[email protected]>
  • Loading branch information
jmfrancois and Alexandre Amalric authored Nov 29, 2023
1 parent 24bcb17 commit 795a12e
Show file tree
Hide file tree
Showing 25 changed files with 94 additions and 124 deletions.
5 changes: 5 additions & 0 deletions .changeset/soft-plums-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@talend/react-components': patch
---

fix: security issue on regexp
7 changes: 7 additions & 0 deletions .changeset/swift-flies-drum.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@talend/react-cmf': patch
---

fix: withoutHOC regex

report says Polynomial regular expression used on uncontrolled data
18 changes: 16 additions & 2 deletions packages/cmf/__tests__/settings.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import { render, screen } from '@testing-library/react';
import { Provider } from 'react-redux';
import { generateDefaultViewId, mapStateToViewProps, WaitForSettings } from '../src/settings';

import { render, screen } from '@testing-library/react';

import { mock } from '../src';
import {
generateDefaultViewId,
mapStateToViewProps,
WaitForSettings,
withoutHOC,
} from '../src/settings';

describe('settings', () => {
describe('mapStateToViewProps', () => {
Expand Down Expand Up @@ -62,6 +69,7 @@ describe('settings', () => {
expect(generateDefaultViewId()).toBe(undefined);
});
});

describe('WaitForSettings', () => {
it('should display using loader if state settings is not initialized', () => {
const state = mock.store.state();
Expand Down Expand Up @@ -99,4 +107,10 @@ describe('settings', () => {
expect(() => screen.getByText('loading')).toThrow();
});
});

describe('withoutHOC', () => {
it('should remove all HOC prefix', () => {
expect(withoutHOC('Connect(CMF(Container(MyComponent)))')).toBe('MyComponent');
});
});
});
5 changes: 3 additions & 2 deletions packages/cmf/src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
* Internal. All stuff related to the settings handling in CMF.
* @module react-cmf/lib/settings
*/
import PropTypes from 'prop-types';
import { connect } from 'react-redux';

import memoize from 'lodash/memoize';
import PropTypes from 'prop-types';

/**
* if viewId is undefined, try to generate a meaningfull one
Expand All @@ -29,7 +30,7 @@ export function generateDefaultViewId(viewId, componentName, componentId) {
* @param {String} viewId Connect(CMF(Container(MyComponent)))
* @return {String} MyComponent
*/
function withoutHOC(componentName) {
export function withoutHOC(componentName) {
return componentName.match(/.*\((.*?)\)/)[1];
}

Expand Down
14 changes: 7 additions & 7 deletions packages/components/src/FormatValue/FormatValue.component.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { useTranslation } from 'react-i18next';

import classNames from 'classnames';
import { escapeRegExp } from 'lodash';
import PropTypes from 'prop-types';

import Icon from '../Icon';
import I18N_DOMAIN_COMPONENTS from '../constants';
import Icon from '../Icon';

import theme from './FormatValue.module.scss';

export const REG_EXP_LEADING_TRAILING_WHITE_SPACE_CHARACTERS = /(^\s*)?([\s\S]*?)(\s*$)/;
const REG_EXP_REPLACED_WHITE_SPACE_CHARACTERS = /(\t| |\n)/g;
const REG_EXP_CAPTUR_LINE_FEEDING = /(\n)/g;
const REG_EXP_LINE_FEEDING = /\n/;
Expand Down Expand Up @@ -104,14 +104,14 @@ export function FormatValueComponent({ value, className }) {

let formattedValue = value;
if (typeof value === 'string') {
const hiddenCharsRegExpMatch = value.match(REG_EXP_LEADING_TRAILING_WHITE_SPACE_CHARACTERS);
const regExp = new RegExp(`(${escapeRegExp(value.trim())})`);
const hiddenCharsRegExpSplit = value.split(regExp);
if (
hiddenCharsRegExpMatch[1] ||
hiddenCharsRegExpMatch[3] ||
hiddenCharsRegExpSplit[0] ||
hiddenCharsRegExpSplit[2] ||
REG_EXP_LINE_FEEDING.test(value)
) {
formattedValue = hiddenCharsRegExpMatch
.slice(1)
formattedValue = hiddenCharsRegExpSplit
.flatMap((flatMapValue, index) => flatMapValue?.split(SPLIT_REGEX[index]))
.filter(isEmptyCharacter)
.map((mappedValue, index) => replaceCharacterByIcon(mappedValue, index, t));
Expand Down
2 changes: 1 addition & 1 deletion packages/jsfc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"watch": "webpack --watch",
"dist-untested": "webpack --config webpack.config.dist.js",
"test:cov": "npm run test",
"test": "echo need to fix all tests before"
"test": "talend-scripts test"
},
"author": "json-schema-form",
"contributors": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Takes a titleMap in either object or list format and returns one
// in the list format.
export default function(titleMap: Array<any>, originalEnum?: any) {
export default function (titleMap: Array<any>, originalEnum?: any) {
if (!Array.isArray(titleMap)) {
const canonical = [];
if (originalEnum) {
Expand Down
16 changes: 8 additions & 8 deletions packages/jsfc/src/index.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import tv4index from 'tv4';

// eslint-disable-next-line prettier/prettier
import canonicalTitleMapImp from './lib/canonical-title-map';
import * as schemaDefaultsImp from './schema-defaults';
// eslint-disable-next-line prettier/prettier
import * as schemaDefaultsImp from './lib/schema-defaults';
import * as sfPathImp from './sf-path';
// eslint-disable-next-line prettier/prettier
import * as sfPathImp from './lib/sf-path';
import canonicalTitleMapImp from './canonical-title-map';

export { merge } from './lib/merge';
export { select } from './lib/select';
export { jsonref } from './lib/resolve';
export { traverseSchema, traverseForm } from './lib/traverse';
export { validate } from './lib/validate';
export { merge } from './merge';
export { select } from './select';
export { jsonref } from './resolve';
export { traverseSchema, traverseForm } from './traverse';
export { validate } from './validate';

export const sfPath = sfPathImp;
export const schemaDefaults = schemaDefaultsImp;
Expand Down
38 changes: 0 additions & 38 deletions packages/jsfc/src/lib/sf-path.spec.js

This file was deleted.

24 changes: 0 additions & 24 deletions packages/jsfc/src/lib/traverse.spec.js

This file was deleted.

8 changes: 4 additions & 4 deletions packages/jsfc/src/lib/merge.js → packages/jsfc/src/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,26 @@ export function merge(
//simple case, we have a "...", just put the formItemRest there
if (stdForm.form && idxRest !== -1) {
let formKeys = form
.map(function(obj) {
.map(function (obj) {
if (typeof obj === 'string') {
return obj;
} else if (obj.key) {
return obj.key;
}
})
.filter(function(element) {
.filter(function (element) {
return element !== undefined;
});

formItemRest = formItemRest.concat(
stdForm.form
.map(function(obj) {
.map(function (obj) {
let isInside = formKeys.indexOf(obj.key[0]) !== -1;
if (!isInside) {
return obj;
}
})
.filter(function(element) {
.filter(function (element) {
return element !== undefined;
}),
);
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as JsonRefs from './../../lib/json-refs-standalone';
import * as JsonRefs from './../lib/json-refs-standalone';

export function jsonref(schema, callBack) {
let promise = new Promise(function(resolve, reject) {
let promise = new Promise(function (resolve, reject) {
JsonRefs.resolveRefs(schema, {
filter: ['relative', 'local', 'remote'],
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ describe('resolve.js', () => {
type: 'object',
properties: {
relative: {
$ref:
'https://raw.githubusercontent.com/json-schema-org/json-schema-org.github.io/master/geo',
$ref: 'https://raw.githubusercontent.com/json-schema-org/json-schema-org.github.io/master/geo',
},
},
};
Expand Down Expand Up @@ -112,7 +111,7 @@ describe('resolve.js', () => {
});

it('should resolve relative json-ref via callback', done => {
jsonref(schema, function(error, resolved) {
jsonref(schema, function (error, resolved) {
if (error) done(error);
resolved.properties.storage.oneOf[0].properties.should.have.property('device');
done();
Expand All @@ -121,7 +120,7 @@ describe('resolve.js', () => {

//I believe this only fails in phantomjs due to https://github.com/ariya/phantomjs/issues/11195
it('should resolve remote json-ref via callback', done => {
jsonref(remote, function(error, resolved) {
jsonref(remote, function (error, resolved) {
if (error) done(error);
//resolved.properties.relative.latitude.type.should.equal('number');
done();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import chai from 'chai';
import { describe, it } from 'mocha';
import { select } from './select';

chai.should();

describe('select.js', () => {
const data = {
name: 'Freddy',
Expand Down Expand Up @@ -36,28 +32,31 @@ describe('select.js', () => {
};

it('should provide a function for getting and setting an object value', () => {
select.should.be.an('function');
expect(typeof select).toBe('function');
});

describe('select', () => {
it('should get a value from an object', () => {
let value = select('frienemies[1].weapon.boomstick.method[0]', data);
value.should.eq('boom');
expect(value).toBe('boom');
});

it('should set a value on an object', () => {
let value = select('weapon.glove.method[1]', data, 'slice');
data.weapon.glove.method.should.be.deep.equal(['stab', 'slice']);
expect(value).toBe('slice');
expect(data.weapon.glove.method[1]).toBe('slice');
expect(data.weapon.glove.method).toEqual(['stab', 'slice']);
});

it('should create any undefined objects or arrays in the path when setting a value', () => {
let data = {};
let value = select('property.array[1].value', data, 'something');
data.should.be.deep.equal({
expect(data).toMatchObject({
property: {
array: [, { value: 'something' }],
},
});
expect(value).toBe('something');
});
});
});
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import chai from 'chai';
import { describe, it } from 'mocha';
import { defaultForm, createDefaults } from './schema-defaults';

chai.should();

describe('schema-defaults.js', () => {
it('should hold functions for generating a default form schema from defaults it creates', () => {
defaultForm.should.be.an('function');
createDefaults.should.be.an('function');
expect(typeof defaultForm).toBe('function');
expect(typeof createDefaults).toBe('function');
});

describe('createDefaults', () => {
it('should create default rules', () => {
const rules = createDefaults();
rules.should.be.an('object');
expect(typeof rules).toBe('object');
});
});

Expand Down Expand Up @@ -55,7 +51,6 @@ describe('schema-defaults.js', () => {
},
},
};

const form = [
{
title: 'Name',
Expand Down Expand Up @@ -199,9 +194,8 @@ describe('schema-defaults.js', () => {
],
},
];

const f = defaultForm(schema, createDefaults());
f.form.should.be.deep.equal(form);
expect(f.form).toMatchObject(form);
});
});
});
10 changes: 10 additions & 0 deletions packages/jsfc/src/sf-path.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { parse, stringify, normalize, name } from './sf-path';

describe('sf-path.js', () => {
it('should hold functions for working with object paths and keys', () => {
expect(typeof parse).toBe('function');
expect(typeof stringify).toBe('function');
expect(typeof normalize).toBe('function');
expect(typeof name).toBe('function');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function name(key: Array<string>, separator?: string, formName = '', omit
let fieldSeparator = separator || '-';

if (omitNumbers) {
fieldKey = fieldKey.filter(function(currentKey: any) {
fieldKey = fieldKey.filter(function (currentKey: any) {
return typeof currentKey !== 'number';
});
}
Expand Down
Loading

0 comments on commit 795a12e

Please sign in to comment.