hasOwnProperty usage #1707

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

Projects

None yet

4 participants

@isaacs
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
isaacs commented Sep 15, 2011

Should be fixed in 0.4 as well as 0.5

@felixge
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
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
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
Member
indutny commented Sep 15, 2011

Ok, you persuaded me. fix is coming soon

@isaacs isaacs added a commit that closed this issue Sep 15, 2011
@isaacs isaacs Fix #1707 hasOwnProperty usage 98990b9
@isaacs isaacs closed this in 98990b9 Sep 15, 2011
@isaacs isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Sep 15, 2011
@isaacs isaacs Fix #1707 hasOwnProperty usage
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.
138c617
@indutny indutny added a commit to indutny/node that referenced this issue Sep 15, 2011
@indutny indutny [debugger] fix for #1707 83fe05b
@indutny
Member
indutny commented Sep 15, 2011
@isaacs isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Sep 15, 2011
@isaacs isaacs Fix #1707 hasOwnProperty usage
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.
e06ce75
@tshinnic

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?

@indutny indutny added a commit to indutny/node that referenced this issue Sep 23, 2011
@indutny indutny [debugger] fix for #1707 f88f445
@ry ry added a commit that referenced this issue Sep 23, 2011
@indutny @ry indutny + ry debugger fix for #1707 43cb4ec
@dead-horse dead-horse added a commit to node-app-engine/sandbox that referenced this issue Dec 27, 2013
@dead-horse dead-horse fix hasOwnProperty usage 2579e90
@dead-horse dead-horse referenced this issue in node-app-engine/sandbox Dec 27, 2013
Merged

fix hasOwnProperty usage #5

@sam-github sam-github pushed a commit to sam-github/node-v0.x-archive that referenced this issue Jul 18, 2015
@rvagg rvagg 2015-05-24 io.js v2.1.0 Release
PR-URL: nodejs/node#1777

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) #1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) #1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) #1699 #1768 #1779.
* npm: Upgraded to v2.10.1, release notes can be found in
  https://github.com/npm/npm/releases/tag/v2.10.1 and
  https://github.com/npm/npm/releases/tag/v2.10.0.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) #1749.
4d8f4d5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment