Re: [PATCH] upnp-info generating false positives

[Patrik Karlsson <patrik () cqure net>]

On 18 nov 2010, at 15.26, Thomas Buchanan wrote:

> On 11/18/2010 01:15 AM, Patrik Karlsson wrote:
> > On 17 nov 2010, at 19.57, Thomas Buchanan wrote:
> >
> > > Hello. In the current SVN version of nmap, the upnp-info script is
> > > generating false positives, marking filtered ports as open when the
> > > connection times out. The issue appears to be in the new upnp.lua
> > > library, and causes an assertion failure, as shown below. The attached
> > > patch corrects the issue for me.
> >
> > Thanks Thomas! I've applied the patch and commited the change as r21090.
>
> Great!
>
> > > One other issue that I noticed in testing the new version of this
> > > script. The previous (monolithic) version of the script would retrieve
> > > the IP address of the HTTP server that serves up the XML files that UPNP
> > > uses, and if that IP address was different from the currently tested IP
> > > address, would generate a warning that the two differed, then go ahead
> > > and try to retrieve the XML file using the original IP address. This
> > > seemed to work fairly well for devices that have the UPNP service
> > > available on the Internet-facing network interface, but would then
> > > return the IP address of the internal NIC in the address of the
> > > webserver where the XML is stored. This was a feature that I found
> > > useful, as it would occasionally disclose information about the internal
> > > network architecture. I noticed that the new version of the script
> > > doesn't exhibit this behavior any longer. Would there be any interest
> > > in a patch to restore this capability, perhaps through a script
> > > argument?
> >
> > I removed this because the "broadcast" version of the script sends it's
> > request to 239.255.255.250.
> > In that case the IP would always mismatch and the script would
> > incorrectly try to query the multicast address for the XML file.
> > It could be restored for the unicast version of the script though. Maybe
> > it should be default behavior to check the same IP and let a script
> > argument override it?
>
> Yes, that was what I had in mind.  I have a rough patch, but will need to clean it up a bit before sending.
>
> Something else that I noticed while I was digging into this is that the current version of the library combines the 
> decoding of the UPNP response with the retrieval of the XML files.  This can lead to false negatives in the situation 
> where the web server for the XML file is not available, either because it is listening on a different interface, or 
> because other network conditions prevent us from accessing it.  When the script fails to download the XML file, it 
> returns an error, without alerting us to the fact that the UDP service was responsive.  

I had some great idea on moving as much of the code as possible from the old script into the library in order to reduce 
the risk of screw up.
Well, it didn't turn out as great as I hoped ....

> How would you feel about me refactoring the Comm.decodeResponse into a couple of different functions, perhaps 
> Comm.decodeResponse, which simply extracts the server and location information from the UDP response, and 
> Comm.retrieveXML which would then perform the HTTP GET for the XML file?  This would allow the script to go ahead and 
> mark the UDP port as open, and display whatever information we gathered from it, whether the XML file was available 
> or not.

Sounds great! Please do so!

> > //Patrik
> >
> > --
> > Patrik Karlsson
> > http://www.cqure.net
> > http://www.twitter.com/nevdull77
>
> Thanks,
>
> Thomas
> _______________________________________________
> Sent through the nmap-dev mailing list
> http://cgi.insecure.org/mailman/listinfo/nmap-dev
> Archived at http://seclists.org/nmap-dev/


Thanks.
P

--
Patrik Karlsson
http://www.cqure.net
http://www.twitter.com/nevdull77





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