Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
fix: prevent prototype pollution in rare error-cases (#923)
Browse files Browse the repository at this point in the history
If a user-provided search parameter is used to instantiate search parameters, it was possible to construct it in such a way that `constructor.prototype` is attempted to be written. That throws an error, but if the error would be caught, the resulting injection still happened.

This PR fixes that (small) vulnerability by ensuring `constructor`, is skipped, just like `__proto__`.

fixes #922

This is similar/a follow-up to #880
  • Loading branch information
Haroenv authored Jan 9, 2023
1 parent 0711861 commit 7ae16ea
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/functions/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ function _merge(target, source) {
for (var key in source) {
if (
!Object.prototype.hasOwnProperty.call(source, key) ||
key === '__proto__'
key === '__proto__' ||
key === 'constructor'
) {
continue;
}
Expand Down
13 changes: 13 additions & 0 deletions test/spec/algoliasearch.helper/constructor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

var algoliasearchHelper = require('../../../index');

test('not vulnerable to prototype pollution', () => {
try {
algoliasearchHelper({}, '', {constructor: {prototype: {test: 123}}});
} catch (e) {
// even if it throws an error, we need to be sure no vulnerability happens
}

expect({}.test).toBeUndefined();
});
12 changes: 12 additions & 0 deletions test/spec/functions/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,15 @@ it('does not pollute the prototype', () => {

expect({}.polluted).toBe(undefined);
});

it('does not pollute the prototype in error condition', () => {
expect({}.polluted).toBe(undefined);

try {
merge({}, {'constructor': {'prototype': {'polluted': 'vulnerable to PP'}}});
} catch (e) {
// ignore
}

expect({}.polluted).toBe(undefined);
});

0 comments on commit 7ae16ea

Please sign in to comment.