Fix prototype pollution issue #529
Conversation
Codecov Report
@@ Coverage Diff @@
## master #529 +/- ##
==========================================
+ Coverage 82.34% 82.36% +0.02%
==========================================
Files 36 36
Lines 1756 1758 +2
==========================================
+ Hits 1446 1448 +2
Misses 310 310
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -172,7 +172,9 @@ Subsegment.prototype.addMetadata = function(key, value, namespace) { | |||
| this.metadata[ns] = {}; | |||
| } | |||
|
|
|||
| this.metadata[ns][key] = value !== null && value !== undefined ? value : ''; | |||
| if (ns !== '__proto__') { | |||
There was a problem hiding this comment.
Do we want to check for the string literal '__proto__' or the object __proto__?
E.g. what would happen if I tried to call segment.addMetadata('key', 'value', __proto__)? Is this something we want to prevent?
There was a problem hiding this comment.
That's a great callout and a case I tested as well!
On line 159 above there's a check that enforces a string type on the namespace object:
if (namespace && typeof namespace !== 'string')
in which case an error would be logged and the method returns. This ensures that if __proto__ gets passed in, which is not a string, this line would never be reached and thus would not be an issue.
There was a problem hiding this comment.
Got it! Thanks for explaining
| @@ -172,7 +172,9 @@ Subsegment.prototype.addMetadata = function(key, value, namespace) { | |||
| this.metadata[ns] = {}; | |||
| } | |||
|
|
|||
| this.metadata[ns][key] = value !== null && value !== undefined ? value : ''; | |||
| if (ns !== '__proto__') { | |||
There was a problem hiding this comment.
Got it! Thanks for explaining
Issue #, if available:
Description of changes:
This change fixes the possible prototype pollution in the
addMetadatamethod in segment.js and subsegment.js and adds corresponding unit tests.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.