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

feat: add --test flag to the ronin-exploits CLI #123

Merged

Conversation

flavorjones
Copy link
Contributor

@flavorjones flavorjones commented May 19, 2024

Currently, to test whether a target is vulnerable, users need to run something like:

ronin-exploits --file=path/to/exploit.rb --dry-run --irb

and then run "test" from the REPL.

This feature would allow users to instead run:

ronin-exploits --file=path/to/exploit.rb --test

Printed output looks like one of these lines, depending on the return type:

[+] Vulnerable: <test result message>
[-] NotVulnerable: <test result message>
[~] Unknown: <test result message>
[!] Unexpected: <other result type>

What drove me to submit this feature is my thought that a flag like this would have sped up feedback loops while I was iterating on https://github.com/ronin-rb/community-pocs/blob/main/exploits/activemq/CVE-2023-46604.rb

Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

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

Good idea. The logic should be moved to the Run command as it's own method, since it's printing to stdout. Also, should update the ## Options documentation above the Run class and the man/ronin-exploits-run.1.md markdown man-page to mention the new option.

Also, since this is a new feature, it should be based off of the 1.1.0 version branch.

This did give me another idea about calling perform_test before perform_build and raising an exception if NotVulnerable was returned, to prevent sending an exploit to a non-vulnerable target.

lib/ronin/exploits/exploit.rb Outdated Show resolved Hide resolved
lib/ronin/exploits/exploit.rb Outdated Show resolved Hide resolved
lib/ronin/exploits/exploit.rb Outdated Show resolved Hide resolved
@flavorjones flavorjones changed the base branch from main to 1.1.0 May 20, 2024 01:34
@flavorjones flavorjones force-pushed the flavorjones-add-test-flag-to-cli branch from 3405536 to 740de65 Compare May 20, 2024 01:34
@flavorjones
Copy link
Contributor Author

@postmodern I've rebased onto 1.1.0 and made most of the requested changes.

I haven't tackled the "precheck" functionality you suggested mostly because I wanted to get agreement on what the feature should look like. What would you think of this:

  • by default, invoke run_test as you suggest, raising ExploitFailed if it returns NotVulnerable
  • allow a --no-precheck flag to skip the test
  • allow a --test flag to ONLY run the test

WDYT?

@flavorjones flavorjones force-pushed the flavorjones-add-test-flag-to-cli branch 2 times, most recently from b983244 to a27900a Compare May 20, 2024 01:42
Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

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

The -D short flag disappeared, and should call exploit.perform_test. Other than that, looks good.

lib/ronin/exploits/cli/commands/run.rb Outdated Show resolved Hide resolved
lib/ronin/exploits/cli/commands/run.rb Outdated Show resolved Hide resolved
lib/ronin/exploits/cli/commands/run.rb Show resolved Hide resolved
lib/ronin/exploits/cli/commands/run.rb Outdated Show resolved Hide resolved
man/ronin-exploits-run.1.md Show resolved Hide resolved
print_error "Unexpected result: #{result.inspect}"
end

result
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to return the result here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured it was better to return it in case the caller wanted to act on the result, rather than return the case statement's value.

@postmodern
Copy link
Member

@postmodern I've rebased onto 1.1.0 and made most of the requested changes.

I haven't tackled the "precheck" functionality you suggested mostly because I wanted to get agreement on what the feature should look like. What would you think of this:

  • by default, invoke run_test as you suggest, raising ExploitFailed if it returns NotVulnerable
  • allow a --no-precheck flag to skip the test
  • allow a --test flag to ONLY run the test

WDYT?

I'll think about this some more than open another issue. We could explicitly call exploit.perform_test in run_exploit and print an error message if NotVulnerable is returned (seems reasonable, imo). Or we could add that check into Exploit#exploit and raise an ExploitFailed exception, which is then rescued/printed in Run#run_exploit; this goes back to the age old debate "is this really an exceptional state?".

@postmodern
Copy link
Member

@flavorjones see issue #124. It doesn't necessary have to be implemented in 1.1.0, and could be postponed until after 1.1.0 is released; unless you need this functionality ASAP.

@flavorjones flavorjones force-pushed the flavorjones-add-test-flag-to-cli branch from a27900a to 9395e3d Compare May 20, 2024 18:41
@flavorjones
Copy link
Contributor Author

@postmodern I've addressed your comments except for the -D which I removed because it was being clobbered by the --debug short flag -D. Let me know if you want --dry-run to own that flag.

@postmodern
Copy link
Member

@flavorjones oh good catch! I'll rename -D, --debug to -d, --debug in main.

@postmodern
Copy link
Member

@flavorjones fixed in the main branch (0f84934) and rebased the 1.1.0 branch to include the fix. Will also be released in the upcoming 1.0.5 patch release.

Currently, to test whether a target is vulnerable, users need to run
something like:

    ronin-exploits --file=path/to/exploit.rb --dry-run --irb

and then run "test" from the REPL.

This feature would allow users to instead run:

    ronin-exploits --file=path/to/exploit.rb --test

Printed output looks like one of these lines, depending on the return
type:

    [+] Vulnerable: <test result message>
    [-] NotVulnerable: <test result message>
    [~] Unknown: <test result message>
    [!] Unexpected: <other result type to_s>
@flavorjones flavorjones force-pushed the flavorjones-add-test-flag-to-cli branch from 9395e3d to 1750fe5 Compare May 21, 2024 21:36
@flavorjones
Copy link
Contributor Author

@postmodern OK, I've rebased onto latest 1.1.0, un-removed the -D flag, and updated the code to call @exploit.perform_test.

@flavorjones flavorjones requested a review from postmodern May 21, 2024 21:37
@postmodern postmodern merged commit b073d47 into ronin-rb:1.1.0 May 21, 2024
7 checks passed
postmodern added a commit that referenced this pull request May 21, 2024
* Added the `-T,--test` option to `ronin-exploits run` to allow only running
  `Exploit#perform_test` to determine if the target is vulnerable or not:

      $ ronin-exploits --file=path/to/exploit.rb --test -p host=example.com ...

* Printed output looks like one of these lines, depending on the return type:

      [+] Vulnerable: <test result message>
      [-] NotVulnerable: <test result message>
      [~] Unknown: <test result message>
      [!] Unexpected: <other result type to_s>

---------

Co-authored-by: Postmodern <postmodern.mod3@gmail.com>
@postmodern
Copy link
Member

postmodern commented May 21, 2024

Oops, made a merge commit instead of a squash merge. Manually re-squash-merged the commits and made sure to preserve your authorship. 😬

@flavorjones flavorjones deleted the flavorjones-add-test-flag-to-cli branch May 22, 2024 18:33
postmodern added a commit that referenced this pull request Jun 12, 2024
* Added the `-T,--test` option to `ronin-exploits run` to allow only running
  `Exploit#perform_test` to determine if the target is vulnerable or not:

      $ ronin-exploits --file=path/to/exploit.rb --test -p host=example.com ...

* Printed output looks like one of these lines, depending on the return type:

      [+] Vulnerable: <test result message>
      [-] NotVulnerable: <test result message>
      [~] Unknown: <test result message>
      [!] Unexpected: <other result type to_s>

---------

Co-authored-by: Postmodern <postmodern.mod3@gmail.com>
postmodern added a commit that referenced this pull request Jun 23, 2024
* Added the `-T,--test` option to `ronin-exploits run` to allow only running
  `Exploit#perform_test` to determine if the target is vulnerable or not:

      $ ronin-exploits --file=path/to/exploit.rb --test -p host=example.com ...

* Printed output looks like one of these lines, depending on the return type:

      [+] Vulnerable: <test result message>
      [-] NotVulnerable: <test result message>
      [~] Unknown: <test result message>
      [!] Unexpected: <other result type to_s>

---------

Co-authored-by: Postmodern <postmodern.mod3@gmail.com>
postmodern added a commit that referenced this pull request Jun 28, 2024
* Added the `-T,--test` option to `ronin-exploits run` to allow only running
  `Exploit#perform_test` to determine if the target is vulnerable or not:

      $ ronin-exploits --file=path/to/exploit.rb --test -p host=example.com ...

* Printed output looks like one of these lines, depending on the return type:

      [+] Vulnerable: <test result message>
      [-] NotVulnerable: <test result message>
      [~] Unknown: <test result message>
      [!] Unexpected: <other result type to_s>

---------

Co-authored-by: Postmodern <postmodern.mod3@gmail.com>
postmodern added a commit that referenced this pull request Jun 29, 2024
* Added the `-T,--test` option to `ronin-exploits run` to allow only running
  `Exploit#perform_test` to determine if the target is vulnerable or not:

      $ ronin-exploits --file=path/to/exploit.rb --test -p host=example.com ...

* Printed output looks like one of these lines, depending on the return type:

      [+] Vulnerable: <test result message>
      [-] NotVulnerable: <test result message>
      [~] Unknown: <test result message>
      [!] Unexpected: <other result type to_s>

---------

Co-authored-by: Postmodern <postmodern.mod3@gmail.com>
postmodern added a commit that referenced this pull request Jun 29, 2024
* Added the `-T,--test` option to `ronin-exploits run` to allow only running
  `Exploit#perform_test` to determine if the target is vulnerable or not:

      $ ronin-exploits --file=path/to/exploit.rb --test -p host=example.com ...

* Printed output looks like one of these lines, depending on the return type:

      [+] Vulnerable: <test result message>
      [-] NotVulnerable: <test result message>
      [~] Unknown: <test result message>
      [!] Unexpected: <other result type to_s>

---------

Co-authored-by: Postmodern <postmodern.mod3@gmail.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.

2 participants