Nmap Development mailing list archives
Re: [PATCH] showHTMLTitle.nse - bugfix and a few improvements
From: David Fifield <david () bamsoftware com>
Date: Tue, 30 Sep 2008 14:08:50 -0600
On Wed, Sep 24, 2008 at 02:38:52AM +0100, jah wrote:
The last time I sent a patch for showHTMLTitle I introduced the ability to follow a redirect from the default page. If the server sends a location header which is not an absolute URI (as per HTML 1.1 spec) a print_debug statement fails, killing script execution. My fault, I should have seen this coming. The attached fixes this by handling relative URIs and making the script much more careful about handling the location header.
This looks like a worthwhile improvement. Two different mechanisms for getting the redirected page are used. With an absolute URI, it's data = http.get_url( loc ) and with a relative URI it's data = http.get( host, port, loc ) Could that be simplified, maybe by having the two cases assign temporary variables representing the new host, port, and loc, then having a common http.get outside of the if block? It doesn't seem right to parse the URL, reconstruct it, and have http.get_url parse it again.
The pattern to match the title tags is less greedy which means that, should there be more than one title in a response (which happens quite often, would you believe), it will return the first of them rather than everything between the opening of the first and the closing of the last. The pattern will also match title tags if they should have attributes of some form.
Those changes look good. I'm sure there are still ways to fool it, like putting title tags inside a comment, or putting a '>' in an attribute to the title. But we have to draw the line for how much of HTML we're willing to parse.
One thing that I didn't do because I thought it was overkill (?) was to use the dns library to check whether a hostname in a location header resolves to the target IP when the hostname does not match either host.targetname or host.name. Instead, it prints debug info.
Yeah, that's a bit much. What's the reason for this and similar changes? - if data.status == 301 or data.status == 302 then + if type( data ) == "table" and type( data.status ) == "number" and tostring( data.status ):match( "30%d" ) and type( data.header ) == "table" and type( data.header.location ) == "string" then Can the response from http.get be anything other than a table? If so, maybe that should be changed in the http library rather than forcing scripts to cope with different return types. I find the new condition less expressive than the old one. What about if data.status ~= nil and tostring( data.status ):match( "30%d" ) and data.header.location ~= nil then At least the very long line should be factored out into an is_redirect function or something. David Fifield _______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://SecLists.Org
Current thread:
- [PATCH] showHTMLTitle.nse - bugfix and a few improvements jah (Sep 23)
- Re: [PATCH] showHTMLTitle.nse - bugfix and a few improvements David Fifield (Sep 30)