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

fix(vapt-scr): use python built-in func #299

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Conversation

abby-ng
Copy link
Contributor

@abby-ng abby-ng commented Oct 17, 2024

Stories

Motivation & Context

  • Flagged out in VAPT source scan as:
    • medium severity finding (stored command injection)
    • low severity finding (stored command argument injection)

Description

  • Same offending code for both findings sys.stdin.readline()

Implication

  • stored command injection
    • The application runs an OS system-level command to complete it's task, rather than via the application code. The command includes untrusted data, that may be controllable by an attacker. This untrusted string may contain malicious system-level commands engineered by an attacker, which could be executed as though the attacker were running commands directly on the application server.In this case, the application reads the command from the database, and passes it as a string to the Operating System. The attacker may be able to load the malicious payload into the database fields beforehand, causing the application to load that command. This unvalidated data is then executed by the OS as a system command, running with the same system privileges as the application.
  • stored command argument injection
    • A potentially tainted value is passed as an argument to an external program, which is executed by code.

Recommendation

  • Same recommendation for both findings
    • Refactor the code to avoid any direct shell command execution. Instead, use platform provided APIs or library calls.
    • If it is impossible to remove the command execution, execute only static commands that do not include dynamic, user-controlled data.
    • Validate all input, regardless of source. Validation should be based on a whitelist: accept only data fitting a specified format, rather than rejecting bad patterns (blacklist). Parameters should be limited to an allowed character set, and non-validated input should be dropped. In addition to characters, check for:
      • Data type
      • Size
      • Range
      • Format
      • Expected values
    • In order to minimize damage as a measure of defense in depth, configure the application to run using a restricted user account that has no unnecessary OS privileges.
    • If possible, isolate all OS commands to use a separate dedicated user account that has minimal privileges only for the specific commands and files used by the application, according to the Principle of Least Privilege.

Implemented Fix

  • as recommended, avoid direct shell command execution. Use python built-in input() function instead

How has this been tested?

@abby-ng abby-ng requested a review from a team October 17, 2024 13:17
@abby-ng abby-ng merged commit 537ba2a into master Oct 18, 2024
4 checks passed
@abby-ng abby-ng deleted the fix--stored-cmd-injection branch October 18, 2024 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants