Skip to content

Update validation for report_vuln#21421

Merged
adfoster-r7 merged 1 commit into
rapid7:masterfrom
adfoster-r7:update-validation-for-report-vuln
May 7, 2026
Merged

Update validation for report_vuln#21421
adfoster-r7 merged 1 commit into
rapid7:masterfrom
adfoster-r7:update-validation-for-report-vuln

Conversation

@adfoster-r7
Copy link
Copy Markdown
Contributor

Continuation of #21314

Verification

Before

When report_vuln is called without a name option:

[-] Auxiliary failed: Msf::ValidationError Failed to report vuln for 10.0.0.10:80 to the database

After

Now:

[-] Auxiliary failed: ArgumentError report_vuln Missing required option :name

@g0tmi1k
Copy link
Copy Markdown
Contributor

g0tmi1k commented May 7, 2026

Idea: What if it would just default to "name" variable, if its not defined? That way, whatever module called it, its used?

@adfoster-r7 adfoster-r7 force-pushed the update-validation-for-report-vuln branch from 6ec123b to 9435bee Compare May 7, 2026 10:55
@adfoster-r7
Copy link
Copy Markdown
Contributor Author

Hm. It looks like there was only one module impacted by this in the codebase, which you've fixed now - so I think it's okay tightening things up here, but let me know your thoughts 🤔

There's the potential that external third party/private modules would be impacted by this change, but I guess there's no change in end user behavior there

@g0tmi1k
Copy link
Copy Markdown
Contributor

g0tmi1k commented May 7, 2026

Not sure if asking me, or others with more exp ;)

My 2 cents is that it makes sense that the module name matches the vuln, and I would rather it be one less thing to think about (and everything matches/lines up) - so I'm all for using defaults (and able to overwrite it if needed/wanted) - keep things consistent.

@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Metasploit Kanban May 7, 2026
@adfoster-r7
Copy link
Copy Markdown
Contributor Author

If we had more modules with this issue, I'd be tempted to have a fallback - but I think we should be good. If any user issues pop up, we can go with the defaults approach. I'm hoping since this always crashed previously, and now there's just a better error message, there shouldn't be any impact 🤞

@g0tmi1k
Copy link
Copy Markdown
Contributor

g0tmi1k commented May 7, 2026

For what its worth, a very rough grep search, 69 modules use name: name for report_vuln.

$ grep -R 'name: name' -B5 | grep 'report_vuln\|name:' | grep report_vuln | wc -l
      69
$ 

@adfoster-r7
Copy link
Copy Markdown
Contributor Author

Thanks for taking a look! 💯

@adfoster-r7 adfoster-r7 merged commit 817d364 into rapid7:master May 7, 2026
47 of 48 checks passed
@adfoster-r7 adfoster-r7 deleted the update-validation-for-report-vuln branch May 7, 2026 12:06
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Metasploit Kanban May 7, 2026
@cdelafuente-r7
Copy link
Copy Markdown
Contributor

Release Notes

This adds extra validation to report_vuln and delete_vuln in Msf::DBManager::Vuln to make sure required field are present and avoid a crash.

@cdelafuente-r7 cdelafuente-r7 added the rn-fix release notes fix label May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rn-fix release notes fix

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants