NetEase / pomelo

A fast,scalable,distributed game server framework for Node.js.
http://pomelo.netease.com
MIT License
11.86k stars 2.9k forks source link

A state poisoning vulnerability in pomelo #1149

Closed xiaofen9 closed 4 years ago

xiaofen9 commented 4 years ago

We found that pomelo allows external control of critical state data. A malicious user-input can corrupt arbitrary methods and attributes in template/game-server/app/servers/connector/handler/entryHandler.js because certain internal attributes can be overwritten via a conflicting name. Hence, a malicious attacker can launch attacks by adding additional attributes to user-input.

A detailed discussion of the vulnerability can be found here. https://github.com/cl0udz/vulnerabilities/tree/master/pomelo-critical-state-manipulation

whtiehack commented 4 years ago

It seems to be a serious problem. I'll test it.

whtiehack commented 4 years ago

This problem does exist and can be simplified to understand and test as this:


var Handler = function (app) {
    this.app = app;

    if (!this.app)
        console.log("error")

};

Handler.prototype.entry = function () {
    console.log('entry', this.app.rpc.auth)
}

let h = new Handler({rpc: {auth: {}}})
console.log('h')
h.entry()
h.constructor({get: {}})
h.entry()
whtiehack commented 4 years ago

image

The temporary solution is to check routeRecord.method in lib/server/server.js globalHandle.

if(routeRecord.method =="constructor"){
  return cb(new Error("unknow method"))
}

There is no need to worry about using pinus, pinus does not have this problem.


class PinusHandler {
    constructor(app) {
        this.app = app;

        if (!this.app)
            console.log("error")

    }

    entry() {
        console.log('entry', this.app.rpc.auth)
    }
}

let ph = new PinusHandler({rpc: {auth: {}}})
console.log('ph')
ph.entry()
ph.constructor({get: {}})
ph.entry()

image

thanks for @xiaofen9

zdhsoft commented 4 years ago

放弃吧,网易的搞的这个,有头没尾的。

alexandreab commented 4 years ago

Is this constructor the single one "exploitable" known? Are there other methods that can be overwritten like this?