Open eric-hemasystems opened 2 years ago
Is this post suggesting this security issue is only relevant if you export alert() to 'jailed?' And the security vulnerability can be managed by not allowing user-defined exports, only user-defined code?
Could 'jailed' fix the CVE by scrubbing the exports and failing if alert() is in the exports?
Is this post suggesting this security issue is only relevant if you export alert() to 'jailed?'
No. See above where it says:
The alert was just used to present the exfiltrated data for demonstration purposes and has nothing to do really with the vulnerability.
And the security vulnerability can be managed by not allowing user-defined exports, only user-defined code?
No. See above where it says:
I don't think the proposed fix to @gpascualg did in https://github.com/asvd/jailed/pull/37 would be sufficient as the Function constructor is not gained via one of the exposed objects passed to the new vm context.
Could 'jailed' fix the CVE by scrubbing the exports and failing if alert() is in the exports?
Since the sandbox escape uses a built-in Node object, this.constructor
, and therefore there is nothing we are passing in I don't think there is a way to prevent the escape. Node.js would have to be changed itself.
Final result is:
If you need to run JavaScript server-side I might suggest try using a different JS engine such as Spidermonkey compiled to wasm run via wasmer. You can either just use the wasmer bin directly or use library bindings to it. Here is a module I made in Ruby that allows me to safely execute JS code server-side.
I was recently notified by Github regarding CVE-2022-23923. The actual description of the issue is a bit odd so wanted to get clarification on its impact. It is described with:
This seems to imply the issue is only present when a remote method named
alert
is exported to the jailed script. The issue seems to be imported from the Sync vulnerability database and that actually includes a POC of the vulnerability. My understanding of the POC leads me to believe the description of the vulnerability is not accurate. It feels like the person who wrote the description is not the same as the person who wrote the POC and the description writer did not understand the POC. Thealert
was just used to present the exfiltrated data for demonstration purposes and has nothing to do really with the vulnerability.My understanding of the POC is it is a variation on the known vulnerability with jailed when running on Node.js and documented on the README. It is getting a
Function
constructor on an object provided by the main context from within the context created by Node.js'svm.runInNewContext
. Then using that to eval code to get access torequire
viaprocess.mainModule
. From there it can do any number of things (in the POC's case it is reading from the filesystem).What makes this POC different from #33 is that it is not using objects passed to the sandbox to get access to this function constructor but instead using
this.constructor
which I am thinking is the constructor for the global object that Node provides the new context? If this is correct I don't think the proposed fix to @gpascualg did in #37 would be sufficient as theFunction
constructor is not gained via one of the exposed objects passed to the new vm context.I'm posting this issue hoping to get feedback from others to confirm my understand which is:
This is just a variation on #33 and therefore represents no more risk than was already documented in the README. jailed on Node.js continues to be broken (only now the proposed fix may be insufficient) but jailed on the web is still valid as it's isolation is handled entirely differently and the constructs used in the POC (
process.mainModule
andrequire
) are not present for JS running in a web browser.