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

Fixed auxiliary/scanner/http/frontpage_login error. #219

Closed
wants to merge 4 commits into from
Closed

Fixed auxiliary/scanner/http/frontpage_login error. #219

wants to merge 4 commits into from

Conversation

silviupopescu
Copy link
Contributor

This is a fix of Bug #6354 of the Metasploit Issue Tracker. There
was an error when trying to write results in the database with an
empty port number.

The code has been modified to write a value if port is empty and
to write the port value without the colon at the beginning if it is
not empty.

Reported by: Nelson LeBlanc
Signed off by: Silviu Popescu silviupopescu1990@gmail.com

This is a fix of Bug #6354 of the Metasploit Issue Tracker. There
was an error when trying to write results in the database with an
empty port number.

The code has been modified to write a value if port is empty and
to write the port value without the colon at the beginning if it is
not empty.

Reported by: Nelson LeBlanc
Signed off by: Silviu Popescu <silviupopescu1990@gmail.com>
@wchen-r7
Copy link
Contributor

wchen-r7 commented Mar 5, 2012

Looks like you figured out :-)

Hey I just realized report_note() doesn't really need a :port key according to the documentation:
http://dev.metasploit.com/documents/api/classes/Msf/DBManager.html#M004963

Also, if you look at that documentation, click on the source -- you'll see that if :port has value 'N/A', you will actually trigger a report_service() call, which seems unnecessary in that particular scenario (since you don't have a port).

So I guess a more appropriate fix would be:

opts = {
:host => target_host,
:proto => 'tcp',
:sname => (ssl ? 'https' : 'http'),
:type => 'FrontPage Author',
:data => "#{info}#{fpauthor}"
}
opts[:port] = port.slice!(0) if not port.empty?
report_note(opts)

I didn't actually test it, so please try it out. Thanks! :-)

@silviupopescu
Copy link
Contributor Author

I'm not sure that's the right solution.
I took a look at the issue as it stands on the Issue Tracker.[1]
After googling the error reported it seems that there is an error when trying to write to a PostgreSQL database.
Basically, port has to be an integer if the underlying database is PostgreSQL.
I changed the value so that it is '0' when port is "".
Do you think it's OK to leave it like this?

[1]http://dev.metasploit.com/redmine/issues/6354

@wchen-r7
Copy link
Contributor

wchen-r7 commented Mar 5, 2012

I don't think having it as '0' is the correct solution at all. You don't have to supply a :port if you don't have one, that's the point.

@wchen-r7
Copy link
Contributor

wchen-r7 commented Mar 5, 2012

On an unrelated note...

port = "0" #This is a string
port = 0 #This is an integer (or "Fixnum" as ruby calls it)

@silviupopescu
Copy link
Contributor Author

I am simply pointing out that PostgreSQL demands a value, unlike other DBMSs, as it is shown by the PGError on the link I provided. Maybe something like -1 would be more appropiate as it is not a port number and it solves the PostgreSQL issue.

About the 0 vs. "0" I read that the database driver automatically calls to_i before storing an integer.

@wchen-r7
Copy link
Contributor

wchen-r7 commented Mar 5, 2012

Have you actually tried it? I've tested report_note() without :port using postgres, and it works fine. Also, confirmed with other two Metasploit devs, you don't need :port.

@wchen-r7
Copy link
Contributor

wchen-r7 commented Mar 5, 2012

Here is a test case for you. This will trigger the exact same error:

    report_note({
        :host => rhost,
        :proto => 'TCP',
        :port => "",
        :type => 'aft_server_info',
        :data => "nothing"
    })

This will not:

    report_note({
        :host => rhost,
        :proto => 'TCP',
        :type => 'aft_server_info',
        :data => "nothing"
    })

@silviupopescu
Copy link
Contributor Author

My apologies if I seemed flamboyant. I actually thought I was right. I could not test the module due to the fact that I do not have an instance of FrontPage to run my tests on, but I played around with irb and you were right (I know ... I'm being Captain Obvious).

As a result, I modified the code to reflect the changes you suggested in a previous comment. Again, I could not test the module in a real world scenario due to the fact that I do not have an instance of FrontPage at my disposal.

Sorry for the trouble caused.

@wchen-r7
Copy link
Contributor

wchen-r7 commented Mar 5, 2012

No worries.

Changes have been made to frontpage_login.rb, thanks for playing.

@wchen-r7 wchen-r7 closed this Mar 5, 2012
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.

None yet

2 participants