Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call js2py.disable_pyimport #97

Closed
franciscouzo opened this issue Mar 22, 2017 · 6 comments
Closed

Call js2py.disable_pyimport #97

franciscouzo opened this issue Mar 22, 2017 · 6 comments

Comments

@franciscouzo
Copy link

My recommendation would be to remove the dependency on js2py, but it looks like you don't want to go that way (Please see this comment), at least disable the pyimport statement, since it gives arbitrary code execution to any website that's scraped. And add the disclaimer that this library runs arbitrary code back to the README (which was removed in this commit).

You should also request a CVE ID.

@Anorov
Copy link
Owner

Anorov commented Mar 23, 2017

Thanks for reporting this. Unfortunately, when the PR to switch to Js2Py was first submitted, I was unaware of all of the library's functionality, including this unfortunate pyimport statement. I did not know it was designed to interoperate so closely and so bidirectionally with Python code.

I skimmed through the Js2Py codebase, and the comment you linked looks accurate. Even after disabling pyimport, the compilation process looks like it may have a large attack surface.

I've reverted back to PyExecJS, with Node as the only supported engine. Changes have been pushed to PyPI, and the warning has been re-added with a note about the vulnerable cfscrape versions. I've also requested a CVE ID from MITRE.

Regarding the new implementation: Node appears to be capable of evaluating untrusted Javascript with its vm module. It does say "The vm module is not a security mechanism. Do not use it to run untrusted code.", but as far as I can tell, there should be no easy way of escaping it when no extra arguments are passed to runInNewContext. Injecting Javascript by escaping the quoted string should be impossible.

If so, the only risk should be DoS (infinite loops, potential memory exhaustion, etc.) and not code execution of any kind. Do you agree and consider this a safe alternative?

@franciscouzo
Copy link
Author

I don't know about the security of V8's VM, so I won't comment on it, but it looks like there are other solutions that you could have a look at (such as https://github.com/patriksimek/vm2)

Removing single quotes, backward slashes and new lines seems enough, but I would also remove NULL bytes (and maybe other non-printable characters) just in case, as PyV8 is written in C++.

@Anorov
Copy link
Owner

Anorov commented Mar 23, 2017

I feel comfortable with Node's VM, given what I've read. vm2 appears to be a wrapper around Node's vm module. Since I'm not passing any extra arguments, I believe I'm using the vm module in its most secure and basic form.

Regarding NULL bytes: the Node executable is used directly; PyV8 is not involved. I don't believe any characters should be able to escape the string context.

@Anorov
Copy link
Owner

Anorov commented Mar 23, 2017

This vulnerability has been assigned CVE-2017-7235.

@ghost
Copy link

ghost commented Mar 23, 2017

Hi @Anorov thankyou for this prompt update.

I see you provide PyExecJS with a line of js to call Node in sandbox mode. Is there anything else I can do to lock down node to just my local python code as I will shortly be installing it which I know little about in order to upgrade cloudflare-scrape.

@Anorov
Copy link
Owner

Anorov commented Mar 23, 2017

@Rutherford No further action should be necessary. After you install Node, code should be run safely and securely.

If you want to ensure complete isolation, you can run cfscrape in a VM or on a server intended purely for scraping (no sensitive data stored on the server, etc.). But this should be unnecessary unless a critical 0-day vulnerability is found in Node.

@Anorov Anorov closed this as completed Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants