Skip to content

Commit 8f066ad

Browse files
authored
Merge pull request #8168 from plotly/bugfix/inherited-property-protect-prototype
Bugfix: fix potential prototype pollution when expanding/merging object paths
2 parents b5e04ce + a53b771 commit 8f066ad

4 files changed

Lines changed: 51 additions & 7 deletions

File tree

‎draftlogs/8168_fix.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fix potential prototype pollution from inherited properties in plot API calls [[#8168](https://github.com/plotly/plotly.js/pull/8168)]

‎src/lib/extend.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ function _extend(inputs, isDeep, keepAllKeys, noArrayCopies) {
7272
input = inputs[i];
7373

7474
for(key in input) {
75-
src = target[key];
75+
src = Object.prototype.hasOwnProperty.call(target, key) ? target[key] : undefined;
7676
copy = input[key];
7777

7878
if(noArrayCopies && isArray(copy)) {

‎src/lib/index.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -887,9 +887,9 @@ lib.objectFromPath = function (path, value) {
887887
var dottedPropertyRegex = /^([^\[\.]+)\.(.+)?/;
888888
var indexedPropertyRegex = /^([^\.]+)\[([0-9]+)\](\.)?(.+)?/;
889889

890-
function notValid(prop) {
890+
function notValid(obj, prop) {
891891
// guard against polluting __proto__ and other internals getters and setters
892-
return prop.slice(0, 2) === '__';
892+
return prop in obj && !Object.prototype.hasOwnProperty.call(obj, prop);
893893
}
894894

895895
lib.expandObjectPaths = function (data) {
@@ -900,7 +900,7 @@ lib.expandObjectPaths = function (data) {
900900
if ((match = key.match(dottedPropertyRegex))) {
901901
datum = data[key];
902902
prop = match[1];
903-
if (notValid(prop)) continue;
903+
if (notValid(data, prop)) continue;
904904

905905
delete data[key];
906906

@@ -912,7 +912,7 @@ lib.expandObjectPaths = function (data) {
912912
datum = data[key];
913913

914914
prop = match[1];
915-
if (notValid(prop)) continue;
915+
if (notValid(data, prop)) continue;
916916

917917
idx = parseInt(match[2]);
918918

@@ -943,11 +943,11 @@ lib.expandObjectPaths = function (data) {
943943
// This is the case where this property is the end of the line,
944944
// e.g. xaxis.range[0]
945945

946-
if (notValid(prop)) continue;
946+
if (notValid(data, prop)) continue;
947947
data[prop][idx] = lib.expandObjectPaths(datum);
948948
}
949949
} else {
950-
if (notValid(key)) continue;
950+
if (notValid(data, key)) continue;
951951
data[key] = lib.expandObjectPaths(data[key]);
952952
}
953953
}

‎test/jasmine/tests/lib_test.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,49 @@ describe('Test lib.js:', function () {
728728
expect(computed).toEqual(expected);
729729
});
730730
*/
731+
732+
it('does not modify inherited objects or the prototype chain', () => {
733+
const payloads = [
734+
'{"toString.call": 1}',
735+
'{"hasOwnProperty.call": 1}',
736+
'{"toString.marker": 1}',
737+
'{"constructor[0].x": 1}',
738+
'{"constructor.prototype.polluted": 1}',
739+
'{"__proto__.polluted": 1}',
740+
'{"x.__proto__.polluted": 1}',
741+
'{"__proto__": {"polluted": 1}}'
742+
];
743+
const targets = [Object.prototype, Object, Object.prototype.toString, Object.prototype.hasOwnProperty];
744+
const ownNames = () => targets.map((target) => Object.getOwnPropertyNames(target));
745+
const before = ownNames();
746+
const originalProcess = window.process;
747+
let after;
748+
let prototypesKept = true;
749+
750+
try {
751+
[undefined, { versions: {} }].forEach((process) => {
752+
window.process = process;
753+
[Lib.expandObjectPaths, (obj) => Lib.extendDeepNoArrays({}, obj)].forEach((fn) => {
754+
payloads.forEach((payload) => {
755+
const input = JSON.parse(payload);
756+
fn(input);
757+
prototypesKept = prototypesKept && Object.getPrototypeOf(input) === Object.prototype;
758+
});
759+
});
760+
});
761+
after = ownNames();
762+
} finally {
763+
window.process = originalProcess;
764+
targets.forEach((target, i) => {
765+
Object.getOwnPropertyNames(target)
766+
.filter((name) => !before[i].includes(name))
767+
.forEach((name) => delete target[name]);
768+
});
769+
}
770+
771+
expect(after).toEqual(before);
772+
expect(prototypesKept).toBe(true);
773+
});
731774
});
732775

733776
describe('coerce', function () {

0 commit comments

Comments
 (0)