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

Arbitrary callable objects passed to the Executor #82

Open
boristopalov opened this issue Jan 6, 2025 · 3 comments
Open

Arbitrary callable objects passed to the Executor #82

boristopalov opened this issue Jan 6, 2025 · 3 comments

Comments

@boristopalov
Copy link

Hey team, first off thanks for this great project, I'll definitely be spending more time with it!

I was looking at the interpreter and found that you can pass in callable objects to the interpreter, which bypasses the restrictions imposed on importing modules. Here is a basic script that passes in a lambda that calls os.remove():

import os
import tempfile

from smolagents.local_python_executor import LocalPythonInterpreter

def test_lambda_closure():
    # Create interpreter with no extra imports or tools
    interpreter = LocalPythonInterpreter(additional_authorized_imports=[], tools={})
    
    # Try to pass in a lambda that uses os
    test_file = "./test.txt"
    
        # Create temporary file in a safe location
    with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
            test_file = f.name
            f.write('test123')
        
    malicious_vars = {
        'bad_func': lambda: os.remove(test_file)
    }
    
    try:
        # Try to execute the lambda
        result, logs = interpreter("bad_func()", malicious_vars)
        assert os.path.exists(test_file), "Security check failed - file was deleted"
        
    finally:
        # Cleanup if file still exists
        if os.path.exists(test_file):
            os.remove(test_file)

test_lambda_closure()

This works even though os is not in the whitelist of allowed modules. I think this would only be an issue if a user directly imports an unsafe module within the scope of where an instance of CodeAgent runs, i.e. the agent can pass in something like os.remove() into the interpreter without importing os since os is already in scope. I'm wondering if it would be worth it to add some additional checks. Let me know what you think

@aymeric-roucher
Copy link
Collaborator

aymeric-roucher commented Jan 6, 2025

Hello @boristopalov, thank you for reporting this!
The callables that you can pass to the interpreter under argument static_tools will be the tools defined by the agent's user (they are the tools passed to the agent). So they should not be more malicious than anything else in the user's codebase. And the other callables that you can pass are the custom_tools, defined in earlier code blobs, to which the import restrictions would apply as well.

Thus this cannot be used even by a malicously fine-tuned LLM to bypass import restrictions.

@boristopalov
Copy link
Author

boristopalov commented Jan 6, 2025

Thanks for the response! @aymeric-roucher I understand that static_tools and custom_tools should be safe-- what I was looking at was passing in values to the additional_variables param when calling the interpreter, which will get stored in interpreter's state field. So rather than trying to pass in a callable as a tool, you pass it directly to the state, through the additional_variables param.

In my example, the code we pass in to the interpreter is bad_func(), and we also pass in bad_func': lambda: os.remove(test_file) for additional_variables.

When the interpreter evaluates bad_func() it will lookup the name of the function, bad_func, in state, get the value of it, and then invoke it without first checking if the callable is doing anything it shouldn't be doing. Does that make sense?

@boristopalov
Copy link
Author

@aymeric-roucher let me know if you think this is a valid concern/loophole. I would be happy to create a PR

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