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

hasOwnProperty usage #1707

Closed
isaacs opened this issue Sep 15, 2011 · 7 comments
Closed

hasOwnProperty usage #1707

isaacs opened this issue Sep 15, 2011 · 7 comments

Comments

@isaacs
Copy link

isaacs commented Sep 15, 2011

These should use Object.prototype.hasOwnProperty.call instead of obj.hasOwnProperty.

lib/_debugger.js:    if (proto.hasOwnProperty(i) && ignored.indexOf(i) === -1) {
lib/module.js:  if (packageCache.hasOwnProperty(requestPath)) {
lib/querystring.js:    if (!obj.hasOwnProperty(k)) {
lib/repl.js:          if (!uniq.hasOwnProperty(c)) {

Causes server crashes when jerks request /foo?hasOwnProperty=x&y=z

@isaacs
Copy link
Author

isaacs commented Sep 15, 2011

Should be fixed in 0.4 as well as 0.5

@felixge
Copy link

felixge commented Sep 15, 2011

I understand how the querystring module might be effected, but what problem do you see with the other modules?

@indutny
Copy link
Member

indutny commented Sep 15, 2011

Debugger shouldn't be affected by this, because Object builtin can't be extended before debugger starts running.

@isaacs
Copy link
Author

isaacs commented Sep 15, 2011

In general, any time there's an object used as a dictionary for arbitrary user-defined strings, it's a bad idea to depend on hasOwnProperty not being overridden.

mkdir -p node_modules
cat >./node_modules/hasOwnProperty <<END
console.log("has pwn properties!")
END

Then doing require("hasOwnProperty") in a node program makes the module system be in a really oddball state.

@indutny
Copy link
Member

indutny commented Sep 15, 2011

Ok, you persuaded me. fix is coming soon

@isaacs isaacs closed this as completed in 98990b9 Sep 15, 2011
isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Sep 15, 2011
If hasOwnProperty is overridden, then calling `obj.hasOwnProperty(prop)`
can fail.  Any time a dictionary of user-generated items is built, we
cannot rely on hasOwnProperty being safe, so must call it from the
Object.prototype explicitly.
indutny added a commit to indutny/node that referenced this issue Sep 15, 2011
@indutny
Copy link
Member

indutny commented Sep 15, 2011

#1712

isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Sep 15, 2011
If hasOwnProperty is overridden, then calling `obj.hasOwnProperty(prop)`
can fail.  Any time a dictionary of user-generated items is built, we
cannot rely on hasOwnProperty being safe, so must call it from the
Object.prototype explicitly.
@tshinnic
Copy link

Umm yeah, all this was the meaning behind #1637
"unguarded fs.watchFile cache statWatchers checking fixed"
where using a filename of 'hasOwnProperty' would blow up the code.

The filename was 'injected' into the object and then later when queried bad things happened. Hey, this is "sql injection" all over again, yes?

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

No branches or pull requests

4 participants