Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multipart flatten includes properties in prototype #438

Open
jamiepoli opened this issue Sep 9, 2024 · 0 comments
Open

Multipart flatten includes properties in prototype #438

jamiepoli opened this issue Sep 9, 2024 · 0 comments

Comments

@jamiepoli
Copy link

The multipart library uses the flatten function, that i've included here for reference:

// flattens nested objects for multipart body
function flatten(object, into, prefix) {
  into = into || {};

  for(var key in object) {
    var prefix_key = prefix ? prefix + '[' + key + ']' : key;
    var prop = object[key];

    if (prop && typeof prop === 'object' && !(prop.buffer || prop.file || prop.content_type))
      flatten(prop, into, prefix_key)
    else
      into[prefix_key] = prop;
  }

  return into;
}

The function uses a for ... in loop to iterate through the properties to add them into the into object. However, a for ... in loop iterates through object prototypes as well. This is not normally a bad thing unless users have defined something in the object prototype that could cause some troubles:

var a = {};
a.__proto__.filename = "/etc/passwd"
// Below code is taken from one of the examples on needle's npm page
var data = {
  buffer: '/home/johnlennon/walrus.png',
  content_type: 'image/png'
};

// the callback is optional, and needle returns a `readableStream` object
// that triggers a 'done' event when the request/response process is complete.
needle
  .post('https://my.server.com/foo', data, { multipart: true })
  .on('readable', function() { /* eat your chunks */ })
  .on('done', function(err) {
    console.log('Ready-o!');
  })

The resultant into object will look like so:
Screenshot 2024-09-09 at 1 51 38 PM

This normally wouldn't be a problem if a user were to stipulate all their properties in their data object. Prototypal lookups will stop if they see that a property is defined at the object level, and will not go up the prototype chain further. However, in cases where those properties are not included when multipart is set to true, could lead to some issues. If a user were to include this library and their application using your library had a vulnerability known as prototype pollution, then a bad user could inject arbitrary properties and wreak havoc. (to be clear, proto pollution is not something inherent to your library, but is posited as a theoretical situation where properties hiding in the object prototype could be found if another user used your library in an application with prototype pollution).

A way to solve this would be to swap out the for ... in loop with Object.entries(), as Object.entries doesn't go up the prototype chain. Therefore, any properties hiding in the prototype will be ignored. I can submit a PR to suggest this change if ignoring prototype-based properties is ideal behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant