Re: [PATCH] showHTMLTitle.nse - bugfix and a few improvements

[jah <jah () zadkiel plus com>]

On 30/09/2008 21:08, David Fifield wrote:
> 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.
>   
I'll have a go at that.  I think it might result in more code, but we'll
see.
>   
> > 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.
>   
You're right, it's fallible.  In the future, we might have use of Sven's
sedusa which could very well improve this.
> 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
>   
My script writing seems to have evolved from testing for a nil value to
being really explicit.  This as a result of often having to fix problems
where an operation on a value breaks the script because the type of the
value was unexpected.  It's almost second nature to me now.
> 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.
The response from http.get should always be a table so you might say
that the long line is a bit paranoid...  Your example above would
probably do the trick without issue.  If I can .. just .. get over the
fear .. untrusted input .. alert! .. must .. test everything ..

I'll have another crack at it.

Regards,

jah


_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org