openhands-disclosure

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

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

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

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.