LeastAuthority / lodestar

GNU Lesser General Public License v3.0
0 stars 0 forks source link

[eth2.0-utils] misc#objectToCamelCase value override edge case #2

Open tacticalchihuahua opened 4 years ago

tacticalchihuahua commented 4 years ago

The function objectToCamelCase accepts an object and converts all property names to camel case format. However, it does not check for name conflicts, so it will blindly override properties if they already exist.

> misc.objectToCamelCase({ some_property: 'foo', someProperty: 'bar' })
{ someProperty: 'foo' }

While this is an edge case - and probably low impact anyway, it might be possible to exploit this in tandem with another unknown vulnerability to manipulate data that is processed or displayed to the user.

Simple to check if there is a name conflict before proceeding to set the value. My suggestion would be to throw if there is a conflict, since this is an unexpected case with no real way to proceed programmatically with any confidence in which value should be used.

Labelling this as a finding for now, since it appears that this is probably used on external/potentially hostile input, but may reclassify as a suggestion later.

tacticalchihuahua commented 4 years ago

Used here: https://github.com/LeastAuthority/lodestar/blob/master/packages/eth2.0-utils/src/yaml/index.ts#L5

tacticalchihuahua commented 4 years ago

https://github.com/LeastAuthority/lodestar/blob/master/packages/eth2.0-utils/src/nodejs/yaml.ts#L5

Look like it's used for loading a YAML file off the user's file system and normalizing the properties. I think this implies that the user is in control of the input, so reclassifying as suggestion.