Skip to content

DOS: js expression make nodejs crash #9295

Closed
Closed
DOS: js expression make nodejs crash#9295

Description

@phra
phra
opened on Oct 26, 2016
  • Version: 6.9.1
  • Platform: Linux kali 4.7.0-kali1-686-pae SMP Debian 4.7.6-1kali1 (2016-10-17) i686 GNU/Linux
  • Subsystem: js built in prototypes

hi,

i was doing some javascript sorcery and i found out that the following js expression make the nodejs kill itself because it's tampering the prototype of Array.

Array.prototype.push = Array.prototype.push.bind(Array.prototype);

e.g.

> Array.prototype.push = Array.prototype.push.bind(Array.prototype)                                                  
[Function: bound push]                                       
> internal/process/next_tick.js:67            
      callback();                
      ^                                                

TypeError: callback is not a function  
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickDomainCallback (internal/process/next_tick.js:122:9)

nodejs is crashing at this line -> https://github.com/nodejs/node/blob/master/lib/internal/process/next_tick.js#L67
i think that making not writable and not configurable the prototypes of built in types can prevent such very evil things.

what do you think about guys?

Activity

  1. cjihrig commented on Oct 26, 2016

    @cjihrig
    cjihrig
    on Oct 26, 2016
    Contributor

    Node shouldn't mess with the built ins if possible. By you doing so, you shouldn't be surprised that things break. We might be able to work around it by storing a reference to the original function before user code can run, but I'm not sure if it's worth it.

  2. phra commented on Oct 26, 2016

    @phra
    phra
    on Oct 26, 2016
    Author

    imho it's a security issue called denial of service.
    i think that the best solution is a default setting that freeze the builtin prototypes, and if someone has to deal with them can just pass a flag to the node executable to disable the default setting and restore the original behaviour.

  3. cjihrig commented on Oct 26, 2016

    @cjihrig
    cjihrig
    on Oct 26, 2016
    Contributor

    Freezing the builtins would probably break a ton of code. Also, how would an attacker execute such an attack? It seems like they would have to already be in a position to execute arbitrary code, in which case they could do much worse.

  4. mscdex commented on Oct 26, 2016

    @mscdex
    mscdex
    on Oct 26, 2016
    Contributor

    I agree with @cjihrig. If an unauthorized person is able to modify your server code, then being worried about such a person adding that kind of breaking code is probably the least of your worries.

  5. phra commented on Oct 27, 2016

    @phra
    phra
    on Oct 27, 2016
    Author

    that's for sure, obviously.

  6. phra
    closed this as completedon Oct 27, 2016

  7. phra commented on Oct 27, 2016

    @phra
    phra
    on Oct 27, 2016
    Author

    just saw that the node exits without a SEGV so RCE is out of scope with this expression.
    btw if there are changes to some prototype that can make v8/node crash with a SEGV, an open road to RCE with the user running the node instance is possible, so outside the v8 vm context. (beyond ASRL, etc)

    for more infos: https://blog.scrt.ch/2013/03/24/mongodb-0-day-ssji-to-rce/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions