Multipart flatten includes properties in prototype
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 45/100
- Issue type
- Bug
- Clarity
- Mostly clear
- Activity status
- Stale
- Tech stack
- javascript
Research direction
Search the multipart implementation for the referenced flatten function and inspect how it builds the multipart body. Add a regression test showing that inherited properties are excluded, then run the existing multipart tests to confirm that own properties and supported file or buffer values still work.
Written by the indexing model from the issue text.
Description
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:
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.
- Dominant language
- JavaScript
- Stars
- 1.6k
- Forks
- 237
- PR merge metrics
- No merged PRs in 30d
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from tomas/needle
-
Difficulty 3/5 1-2 days Newbie friendliness 45/100
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 3/5 1-2 days Newbie friendliness 45/100
-
Difficulty 3/5 1-2 days Newbie friendliness 42/100
-
Difficulty 3/5 1-2 days Newbie friendliness 45/100
Similar issues
-
code-quality refactoring
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
github/gh-aw-firewall#8816 ·
-
integration:quickjs org:external priority:backlog topic:code-interpreter topic:middleware type:feature
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
langchain-ai/deepagents#6450 ·
-
optimization optimization:agents-md-curator
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
githubnext/gh-aw-cao#13143 ·
-
status: needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100