Skip to content

some fixes#2900

Open
doomedraven wants to merge 7 commits intomasterfrom
some_fixes
Open

some fixes#2900
doomedraven wants to merge 7 commits intomasterfrom
some_fixes

Conversation

@doomedraven
Copy link
Collaborator

No description provided.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @doomedraven, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the stability and usability of the administrative tools, particularly those interacting with remote servers via SSH. It focuses on improving the reliability of SSH connections through reuse and better error handling, introduces retry logic for critical operations like file deployment, and provides a new utility for managing SSH known hosts. Additionally, it includes minor but important fixes in database object persistence and adds a crucial validation step in the CAPE processing module to prevent issues with missing target files.

Highlights

  • Robust SSH Connection Management: Implemented thread-safe SSH connection reuse and improved error handling for remote operations, ensuring more reliable communication with servers.
  • Enhanced Remote File Operations: Introduced retry mechanisms for file deployments and refined logging to capture both standard output and errors from remote commands, making file transfers and command executions more resilient.
  • New CLI Option for SSH Key Management: Added a new command-line argument, --remove-ssh-keys, to facilitate the removal of server SSH keys from the local known_hosts file.
  • Database Session Fixes: Corrected the timing of adding machine and task objects to the database session, ensuring proper persistence.
  • Processing Module Validation: Added a check in the CAPE processing module to log an error if a target file is missing, preventing potential data display issues.
Changelog
  • admin/admin.py
    • Removed unused 'POSTPROCESS' import.
    • Introduced 'RETRY' constant for operation retries.
    • Added '--remove-ssh-keys' command-line argument.
    • Updated debug logging configuration to set global logger level.
    • Implemented retry logic for 'deploy_file' operations.
    • Modified 'enumerate_files_on_all_servers' to accept additional arguments.
    • Updated 'restart_service' command to use 'systemctl' and added SSH key removal functionality.
  • lib/cuckoo/common/admin_utils.py
    • Added 'Lock' for thread-safe access to SSH sockets.
    • Improved SSH connection logic to reuse active connections and handle 'NameError' during host lookup.
    • Enhanced error handling for SSH connection failures, logging errors instead of exiting the program.
    • Modified 'execute_command_on_all', 'get_file', 'deploy_file', and 'delete_file' to use 'get_pty=True' for 'exec_command' to capture stderr.
    • Improved logging for remote command execution, including error output.
    • Removed explicit 'ssh.close()' calls in several functions to facilitate connection reuse.
  • lib/cuckoo/core/database.py
    • Adjusted the order of 'self.session.add(machine)' and 'self.session.add(task)' calls to ensure objects are added to the session earlier in their respective creation methods.
  • modules/processing/CAPE.py
    • Added a check to log an error if the target file for processing no longer exists, preventing data display issues on the web GUI.
Activity
  • No human activity (comments, reviews) has been recorded on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several fixes and improvements, including enhanced SSH connection handling in admin_utils.py with connection reuse, a new command-line option to remove SSH keys, and a bug fix in the CAPE.py processing module. However, it critically introduces command injection vulnerabilities in admin_utils.py where user-supplied arguments are interpolated into shell commands without proper escaping; these must be addressed by using shlex.quote(). Additionally, a critical bug exists in the file deployment retry mechanism, and a high-severity typo was found in a logging call.


elif args.enum_all_servers:
enumerate_files_on_all_servers()
enumerate_files_on_all_servers(servers, jumpbox, "/opt/CAPEv2", args.filename)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The enumerate_files_on_all_servers function constructs a shell command by interpolating the filename argument directly into a string. This command is then executed on remote servers via ssh.exec_command. Since filename comes from the --filename command-line argument in admin.py and is not sanitized or escaped, an attacker who can control this argument can achieve arbitrary command execution on the remote servers.

To remediate this, use shlex.quote() to escape the filename before interpolating it into the command string.


if remote_command:
_, ssh_stdout, _ = ssh.exec_command(remote_command)
_, ssh_stdout, ssh_stderr = ssh.exec_command(remote_command, get_pty=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The deploy_file function executes remote_command on remote servers via ssh.exec_command. The remote_command is constructed in file_recon by interpolating the TARGET path, which is derived from the file argument (specifically its basename). If a filename contains shell metacharacters (e.g., ;, $(...), `...`), it can lead to arbitrary command execution on the remote server.

To remediate this, ensure that remote_command is properly escaped using shlex.quote() or that the input filenames are strictly validated.

doomedraven and others added 5 commits February 5, 2026 10:40
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

1 participant