Metasploit-framework: enhance request, more informative error message

[jinq102030]

Steps to reproduce

When run a wrong command (note the extra "." at the end of the module), the error message should be more explicit

msf  > use auxiliary/scanner/http/f5_mgmt_scanner.
[-] Failed to load module: auxiliary/scanner/http/f5_mgmt_scanner.

Expected behavior

Would be nice if MSF could tell user that there is an extra "." at the end of the module or extra ".rb" at the end of module

[h00die]

Would the best method to go about this be to say split on / , and if a . is located in the last string, strip it and anything right of it off?
Python pseudo code:

module_name = module_name.split("/")
if "." in module_name[-1]:
  module_name[-1] = module_name[-1].split(".")[0]
module_name = "/".join(module_name)

I'm not aware of any module names with periods in them, so this would be safe as far as my VERY limited knowledge would go

[jinq102030]

There is a good solution too.

[scriptkittie]

@h00die I think that if this enhancement is going forward, it should handle more than just checking for periods. The method you describe is great, maybe it could check for other things as well, other than just periods?

[h00die]

What did you have in mind? You'd want to be careful to not do something like:

1. go to the directory the module is in
2. check if module name is in there
3. if not, remove the right most character, then check if a module starts with the remaining string.

This could possibly have unintentional consequences. I tried to find an example for 2min, but failed. However, lets pretend there was:

exploits/http/awesome_rce_auth
exploits/http/awesome_rce

Then a user did use exploits/http/awesome_rce_buth, it would keep removing until awesome_rce and hit on an unintended module.

If its just a matter of saying 'valid modules are [a-z0-9_]+' and remove anything else, maybe that would be ok. Especially if a warning was thrown to say "exploits/http/awesome_rce_but....h validated to exploits/http/awesome_rce_buth".

[wvu-r7]

Too much magic can be a bad thing, too.

[scriptkittie]

@h00die I completely agree. if valid modules match that regex then that is alone the lines of what i was thinking. if the module is not found then you could check for invalid characters(such as "." and ",").

As a sidenote, are all module names alphanumeric wth a "/" seperating them?

[bwatters-r7]

Perhaps if the module is not a valid module, then we strip to the module name and reply with the response of a search command on it rather than jumping to the directory (you could fat-finger the directory name).
The problem is that the search is a little restrictive because it does not do the fuzziness you'd want, but that is fixable.

[wvu-r7]

It can also be slower. The use command already tries to instantiate a module by name, and it'll even retry if it doesn't get it the first time. If you're adding filesystem searches to that, it can get miserably slow(er).

Don't get me wrong, I like this idea! I just feel it's headed in an direction that isn't terribly performant in an effort to be nice to the user who fat-fingered the module name in the first place.

@jinq102030: Being the performance testing guy, what do you think?

[bwatters-r7]

If you want performance, then don't misspell the module name.

[wvu-r7]

To be fair, what hasn't been said is that a trailing period is due to copying and pasting a module path, not typing it manually.

[jinq102030]

I don't want to misspell module names, but copying and pasting (different shortcuts on Mac or Ubuntu VM) just play joke on me :-(

[wvu-r7]

@jinq102030: It does to me, too... I've thought about this problem but just resorted to copying and pasting more carefully. I guess y'all are onto something...

[wvu-r7]

So who wants to write the code?

[jinq102030]

For now, it would be sufficient to check if the module entered ends with "." and ".rb", these are the most mistake by absent-minded user who is multi-tasking.

[h00die]

if you point me in a direction, i could take a stab, however, itd help if you said 'heres the line where we check to see if it exists and handle the error where we cant find it'

[jinq102030]

Hi @h00die, I meant to say, if the module name that user entered ends with either "." or ".rb", then just strip it out and use the remaining part as the module name. Very simple change, but useful for those of us who are multi-tasking a lot.

[wvu-r7]

I have a patch.

[jinq102030]

Cool.

[wvu-r7]

Since I've mired myself in this ticket, I'll take responsibility for the implementation. You can judge if it sucks or not. Criticisms always encouraged.

So far, the patch is fast enough for my liking, enforces a module reference name (no modules/ or .rb), and doesn't alter the module loader (which I've decided would be the worst thing to do).

Will PR in a sec.

[h00die]

you can assign it to me, i'm going to get a few things touched up in the next few days so i can handle it

[wvu-r7]

Thanks, @h00die. I've taken this experiment to its logical conclusion and benchmarked each scenario. Are we okay with a ~10 microsecond difference between . or .rb and modules/? :-)

[wvu-r7]

FYI, #4615 is why it's actually slow. #7734 bypasses that. If you know what the user meant, don't attempt the same mistake twice...