Replace exec with wasm_exec #5640

Jflick58 · 2023-06-02T23:47:47Z

This replaces the usage of the insecure exec function with a more secure library called wasm_exec TL;DR: Use chroot jails, wasmtime and a standalone Python3.11 WASM interpreter to safely execute arbitrary code

Some questions before I officially submit this pr:

How critical is it that we support the execution of code containing 3rd-party dependencies? The team behind the wasm interpreter I vendor with the library is working on porting numpy along with some of the c libs needed to enable broader package support.
- If it is critical, then how critical is it to support Python versions below 3.11? Right now the interpreter does support the use of an existing venv for accessing installed 3rd party deps, but the interpreter is only distributed in 3.11 which means I would either a) have to only support 3.11 b) have a 3rd split in the PythonRepl code base (base, AST, Wasm) or b) try to write some hacky and unpythonic code to try to dynamically create a new 3.11-based venv off of the currently active Python environment.
- If it is not critical then is it okay if I refactor the test cases to avoid the tests containing numpy and pandas code? Should we also update the docs to reflect that limitation?

I also strongly welcome feedback on the wasm_exec library if there are implementation ideas that make it more functional.

Fixes #1026
Fixes #5294
Fixes #5388
Fixes https://nvd.nist.gov/vuln/detail/CVE-2023-29374

Who can review?

uogbuji · 2023-06-03T01:33:17Z

Just my opinions here. I've not commit bit & am relatively new to langchain.

This replaces the usage of the insecure exec function with a more secure library called wasm_exec TL;DR: Use chroot jails, wasmtime and a standalone Python3.11 WASM interpreter to safely execute arbitrary code

100% needed, and thanks!

Some questions before I officially submit this pr:

How critical is it that we support the execution of code containing 3rd-party dependencies? The team behind the wasm interpreter I vendor with the library is working on porting numpy along with some of the c libs needed to enable broader package support.

It's better to get this in place first, and then worry about 3rd party deps later. The security concern massively outweighs feature depth.

If it is critical, then how critical is it to support Python versions below 3.11? Right now the interpreter does support the use of an existing venv for accessing installed 3rd party deps, but the interpreter is only distributed in 3.11 which means I would either a) have to only support 3.11 b) have a 3rd split in the PythonRepl code base (base, AST, Wasm) or b) try to write some hacky and unpythonic code to try to dynamically create a new 3.11-based venv off of the currently active Python environment.

I also think it's not worth a lot of effort to support power Python versions. (b) & (c) feel like maintenance burdens, and simplicity is important in security-minded components.

langchain/utilities/python.py

Jflick58 · 2023-06-06T16:46:07Z

@hwchase17 @vowelparrot would love some feedback on this PR. I recognize some of the tests are still failing, but I think that resolution is dependent upon answers to the questions I posed above. Thanks!

Jflick58 · 2023-06-17T19:28:55Z

@hwchase17 @vowelparrot bumping again, hoping to get your thoughts...

hwchase17 · 2023-06-24T05:00:50Z

pyproject.toml

@@ -102,6 +102,7 @@ azure-cognitiveservices-speech = {version = "^1.28.0", optional = true}
 py-trello = {version = "^0.19.0", optional = true}
 momento = {version = "^1.5.0", optional = true}
 bibtexparser = {version = "^1.4.0", optional = true}
+wasm-exec = "^0.1.7"


this should not be required dependeyncy, lets make optional

This seems dependent on the conversation below. Given that this is a big reason for the CVE that are flagging Langchain in different security scans, I'd advocate (for now) to make it a required dependency.

hwchase17 · 2023-06-24T05:01:35Z

langchain/utilities/python.py

+        arbitrary_types_allowed = True
+
+
+def _get_wasm_executor() -> WasmExecutor:


i would probably bias towards a separate util tbh. since this introduces a new dependency

I think this also goes back to the fundamental question of whether this should be an add-in or replace the default exec functionality. If it's the default replacement, then seems like leveraging the python util makes sense.

hwchase17 · 2023-06-24T05:01:45Z

langchain/tools/python/tool.py

@@ -18,7 +18,7 @@


 def _get_default_python_repl() -> PythonREPL:
-    return PythonREPL(_globals=globals(), _locals=None)
+    return PythonREPL(_globals=None, _locals=None)


The goal of this PR is to offer better security in use of Python REPL, and passing in globals() looks like a massive security hole to me. I see a lot of code out there where people expose e.g. their API keys and other secrets as globals in memory space, and those could be a prompt injection target. The rest of your comments, @hwchase17 seem to be towards making a higher security level optional, and I'd reluctantly accept that (for me, higher security should be the default), but if the user does opt for a tighter ship, then that _globals=None is IMO an important part of the picture.

agreed with @uogbuji but also right now the Wasm interpreter doesn't supported globals that can't be represented as a type that can be easily converted to string. Often the result of the globals function call are object types that can't be converted to string.

vercel · 2023-07-03T05:49:12Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
langchain	⬜️ Ignored (Inspect)	Visit Preview		Nov 1, 2023 11:18pm

Jflick58 · 2023-07-03T05:49:58Z

@hwchase17 can you review the responses to the comments and let me know how you want to proceed?

timxieICN · 2023-07-11T17:48:25Z

any update on this feature/PR? Thanks.

hwchase17 · 2023-07-18T06:52:14Z

sorry for the delay here. high level thoughts:

We almost certainly don't want to introduce this as a dependency for the main project
We want to support as flexible of python running for prototyping purposes as possible
We do not want people to be running this in production

I would suggest the following changes:

We will move the current python tools and anything that depends on them into langchain.experimental and make that a separate package. This should remove CVEs from core langchain, while also hopefullying making it clear its experimental, and also allowing for prototyping
I would encourage the creation of a separate langchain_python or the like respository where that uses wasm by default. we'd be happy to support this in a variety of ways (help setting up, help publicizing, help maintaining), but i think we'd prefer it to be a separate repository

thoughts?

Jflick58 · 2023-07-22T17:28:32Z

Responded more in #8092

As far as this goes, I'll probably take these changes and implement as a separate package. How would that jive with the story of langchain.experimental though? Seems confusing to me to have an unsafe and safe way of doing things, both as external packages and possibly publicized by LangChain.

leo-gan · 2023-09-15T01:50:01Z

@Jflick58 Hi , could you, please, resolve the merging issues? After that ping me and I push this PR for the review. Thanks!

Jflick58 · 2023-09-16T20:24:57Z

@leo-gan I thought @hwchase17 decided to move the repl functionality to langchain-experimental to avoid including this security issue in the main langchain install? Happy to work on this PR to include the wasm-exec functionality in the main package, but wanted to confirm. Thanks!

leo-gan · 2023-09-16T23:28:08Z

@Jflick58 If this change suits experimental better, is it possible to start this change again as a new PR in experimental? If you want to do this, please, close this PR.

EliahKagan · 2023-09-16T23:41:53Z

Would it have to be a new PR? Given that langchain and langchain-experimental are both from this repository, could this PR be modified accordingly?

leo-gan · 2023-09-17T00:40:13Z

It is up to you. This PR could be OK 👍

ruan-azevedo · 2023-10-03T15:20:11Z

Hey guys, any updates for this PR?

Jflick58 · 2023-10-03T16:03:55Z

@RuanAzevedo This is still on my to-do list. I'm traveling for work this week but hoping to pick it back up next week.

nfcampos · 2023-10-09T15:45:07Z

libs/langchain/langchain/utilities/python.py

@@ -66,6 +78,6 @@ def run(self, command: str, timeout: Optional[int] = None) -> str:
                p.terminate()


the timeout branch also needs to be updated to pass self.wasm as arg

Why would we pass self.wasm as an arg if it has access to the wasm class attribute?

tabdunabi · 2023-10-10T18:21:24Z

hi @Jflick58, any update on this PR (to resolve this CVE)?

kenziewritescode · 2024-02-27T22:20:30Z

Is there any update on this?

hwchase17 · 2024-05-09T01:02:25Z

closing as the CVE has been addressed through other means. i am supported of a wasm based exec tool in the future

Jflick58 and others added 2 commits June 2, 2023 23:27

replace exec with wasm_exec

de41fb4

Merge branch 'master' into Safe-exec

e71f453

uogbuji reviewed Jun 3, 2023

View reviewed changes

langchain/utilities/python.py Outdated Show resolved Hide resolved

Jflick58 added 3 commits June 3, 2023 20:53

fix spurious print statement

40f80ed

no default globals in base Python repl (parity with ast repl)

1505d5e

fix tests

846eb39

Jflick58 changed the title ~~WIP: Replace exec with wasm_exec~~ Replace exec with wasm_exec Jun 3, 2023

Jflick58 added 2 commits June 3, 2023 21:10

fix poetry.lock

8fa950c

Merge branch 'hwchase17:master' into Safe-exec

f415269

hwchase17 reviewed Jun 24, 2023

View reviewed changes

hwchase17 added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jun 24, 2023

Jflick58 mentioned this pull request Jun 26, 2023

Simulate sandbox execution of bash code or python code. #2301

Closed

fix unit tests

c650459

vercel bot temporarily deployed to Preview July 3, 2023 06:52 Inactive

Jflick58 mentioned this pull request Jul 18, 2023

Prompt injection which leads to arbitrary code execution #7700

Closed

14 tasks

baskaryan added 5 commits August 11, 2023 12:51

revive

6fb671f

rm

1546e1e

import

f991d6d

fmt

897cc22

poetry

3bdaf4e

sfc-gh-jcarroll mentioned this pull request Aug 24, 2023

Add warnings on chat_pandas_df langchain-ai/streamlit-agent#30

Merged

hwchase17 removed the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Oct 4, 2023

nfcampos reviewed Oct 9, 2023

View reviewed changes

tabdunabi mentioned this pull request Oct 10, 2023

Arbitrary code execution in LLMMathChain #8363

Closed

14 tasks

Jflick58 added 7 commits November 1, 2023 15:49

Merge branch 'wasm-exec-experimental' into Safe-exec

02ffdc3

remove previous implementation

998bba5

add python utility back to main lib

7b95d70

formatting fixes.

30976ea

add dependency for extended tests

5a6d811

add testing dependencies

10f7ef2

correct tests

4e7d3ea

hwchase17 closed this Jan 30, 2024

baskaryan reopened this Jan 30, 2024

eyurtsev added 13 security 🤖:security Related to security issues, CVEs labels Mar 13, 2024

hwchase17 closed this May 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace exec with wasm_exec #5640

Replace exec with wasm_exec #5640

Jflick58 commented Jun 2, 2023 •

edited

Loading

uogbuji commented Jun 3, 2023

Jflick58 commented Jun 6, 2023

Jflick58 commented Jun 17, 2023

hwchase17 Jun 24, 2023

Jflick58 Jun 25, 2023

hwchase17 Jun 24, 2023

Jflick58 Jun 25, 2023

hwchase17 Jun 24, 2023

uogbuji Jun 24, 2023 •

edited

Loading

Jflick58 Jun 25, 2023

vercel bot commented Jul 3, 2023 •

edited

Loading

Jflick58 commented Jul 3, 2023

timxieICN commented Jul 11, 2023

hwchase17 commented Jul 18, 2023

Jflick58 commented Jul 22, 2023

leo-gan commented Sep 15, 2023

Jflick58 commented Sep 16, 2023

leo-gan commented Sep 16, 2023

EliahKagan commented Sep 16, 2023 •

edited

Loading

leo-gan commented Sep 17, 2023

ruan-azevedo commented Oct 3, 2023

Jflick58 commented Oct 3, 2023

nfcampos Oct 9, 2023

Jflick58 Nov 1, 2023

tabdunabi commented Oct 10, 2023

kenziewritescode commented Feb 27, 2024

hwchase17 commented May 9, 2024

		arbitrary_types_allowed = True


		def _get_wasm_executor() -> WasmExecutor:

		@@ -66,6 +78,6 @@ def run(self, command: str, timeout: Optional[int] = None) -> str:
		p.terminate()

Replace exec with wasm_exec #5640

Replace exec with wasm_exec #5640

Conversation

Jflick58 commented Jun 2, 2023 • edited Loading

Who can review?

uogbuji commented Jun 3, 2023

Jflick58 commented Jun 6, 2023

Jflick58 commented Jun 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uogbuji Jun 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Jul 3, 2023 • edited Loading

Jflick58 commented Jul 3, 2023

timxieICN commented Jul 11, 2023

hwchase17 commented Jul 18, 2023

Jflick58 commented Jul 22, 2023

leo-gan commented Sep 15, 2023

Jflick58 commented Sep 16, 2023

leo-gan commented Sep 16, 2023

EliahKagan commented Sep 16, 2023 • edited Loading

leo-gan commented Sep 17, 2023

ruan-azevedo commented Oct 3, 2023

Jflick58 commented Oct 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tabdunabi commented Oct 10, 2023

kenziewritescode commented Feb 27, 2024

hwchase17 commented May 9, 2024

Jflick58 commented Jun 2, 2023 •

edited

Loading

uogbuji Jun 24, 2023 •

edited

Loading

vercel bot commented Jul 3, 2023 •

edited

Loading

EliahKagan commented Sep 16, 2023 •

edited

Loading