-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
Create shodan_host.rb #14429
Create shodan_host.rb #14429
Conversation
Natto97
commented
Nov 25, 2020
Thanks for your pull request! Before this can be merged, we need the following documentation for your module: |
Thanks for your pull request! Before this pull request can be merged, it must pass the checks of our automated linting tools. We use Rubocop and msftidy to ensure the quality of our code. This can be ran from the root directory of Metasploit:
You can automate most of these changes with the
Please update your branch after these have been made, and reach out if you have any problems. |
@Natto97 Any update on creating documentation for this PR and updating the module to fix the issues reported by Travis? |
@gwillcox-r7 Update file completed #14511 |
Has been updated, is it like this @gwillcox-r7 |
Hello @Natto97. So I was just reviewing this module and I noticed that we already have a module in the framework right now that performs very similar functionality to your module, specifically https://github.com/rapid7/metasploit-framework/blob/c0b42ff7a2a1b677aaba6acd7ebeefddcffdf38e/modules/auxiliary/gather/shodan_search.rb. That being said during my tests I did notice that the module I linked above will not work on free accounts as the So with all this being said it seems the best option would be to update the module at https://github.com/rapid7/metasploit-framework/blob/c0b42ff7a2a1b677aaba6acd7ebeefddcffdf38e/modules/auxiliary/gather/shodan_search.rb so that it adds a new check to see if the user specified an IP as the search filter and if they did, then adjust the query format to specifically use the new API format to prevent users having to use the paid This will allow us to still gather the same information without having similar code spread across multiple modules, thereby reducing code duplication. In essence we will just be expanding upon the existing module to add in some newer features. |
Is there a difference between the |
I'm actually not sure if the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay very sorry for the confusion here. So what happened was that we were going to try and merge the changes into https://github.com/rapid7/metasploit-framework/blob/c0b42ff7a2a1b677aaba6acd7ebeefddcffdf38e/modules/auxiliary/gather/shodan_search.rb however after a lot of deliberation and analysis we discovered that module heavily relies on the concept of splitting results by pages, which is a pro feature, and the API this module uses doesn't support that feature. Additionally since that module's code is heavily based around the pages feature, its hard to refactor the code to support this new API.
Therefore I think it would be better to leave this module as is and if people have questions as to why some factors might be duplicated we can point them to this discussion. Again sorry for this confusion @Natto97.
Below are a few minor things that I found whilst reviewing the module. I'm going to go ahead and fix these up myself. I'm just leaving them here for your records, however don't worry I'll make sure these are fixed and the module will be landed today once all the changes have been applied and retested to make sure everything still works appropriately.
…tments to the exploit code to handle some edge cases and fix review comments
Release NotesNew module |