Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: [NSE] Improved version of ms-sql-info
From: Patrik Karlsson <patrik () cqure net>
Date: Wed, 26 Jan 2011 19:07:52 +0100

Hi Chris,

First off, great work!
I had the chance to test your new script today and while it worked great against most servers I also ran into the 
following error:

NSE: ms-sql-info against 1.2.3.4:1433 threw an error!
./scripts/ms-sql-info.nse:319: attempt to perform arithmetic on a nil value
stack traceback:
        ./scripts/ms-sql-info.nse:319: in function 'retrieve_version_from_ssnetlib'
        ./scripts/ms-sql-info.nse:414: in function 'process_instance'
        ./scripts/ms-sql-info.nse:470: in function 'process_response'
        ./scripts/ms-sql-info.nse:500: in function <./scripts/ms-sql-info.nse:479>
        (tail call): ?

Unfortunately I don't have any more information or packet traces that I can supply you with.
Some more comments inline.

On 25 jan 2011, at 03.19, Chris Woodbury wrote:

I have taken the ms-sql-info script and made what I hope are
considered to be some improvements. Chief among them, the version
detection is now more reliable, more accurate, and uses a method that
would let the script's categorization be changed from "intrusive" to
"safe" (a big plus for a default script).

This sounds like a great idea, the key words being more reliable, accurate and less intrusive.


The revised script uses the same Discover function from the mssql
library, but, instead of attempting to log in with a blank password
for the "sa" account to check the version, it sends a TDS pre-login
packet and parses the server's version number from the response (the
same method used by SQLPing and by Nmap's own service versioning
probes).

I think this method is far superior as in most cases it will end up with a more accurate result.
However, if possible, I would like to see the pre-auth packet dissected and moved into the library and decoded as much 
as possible rather than relying on retrieving substrings from the response.
I guess either Microsoft, the FreeTDS folks or Wireshark would probably have the needed documentation to achieve this.

This has the advantage of working every time, as long as the
TCP port for the SQL Server instance is accessible (and, if it
weren't, the logging-in method wouldn't work either), and it also
doesn't run the risk of failed login attempts (which are dangerous now
that SQL Server has account lockout policies). Plus, the lost side
functionality is now available in the ms-sql-empty-password script.

This is almost true. One extremely annoying thing I noticed today when I scanned a server with 11 instances  was that I 
had to wait for Nmap to fingerprint the services on all ports before being able to run ms-sql-empty-password against 
them. I aborted the scan and ended up testing quicker manually (I type very quickly (-: ). 
Anyway, I don't believe that this should be considered as a problem with this new revised script.


Now that we have a more reliable way of getting accurate version
information, I also expanded the display of the version information,
so that the script determines the version, the service pack level and
whether additional patches have been installed.

Additionally, as an aid to people who may not want an NSE script to
make connections to ports they did not originally scan, I added a
"browseronly" argument, which will have the script only connect to the
SQL Server Browser service (done by mssql.Helper.Discover). This
limits the accuracy of the version information, but allows tighter
control over what the script is doing.

This sounds good to and the description clearly states the drawbacks.


Also, I took the liberty of removing the "require('target')," since it
wasn't being used and may mislead users into thinking that the script
will add identified instances (which would be great functionality).

I probably missed this when splitting the broadcast code out from the script.


Last but not least, I updated the existing NSEDoc information and
expanded the description.


As I mentioned previously, I'm fairly new to Lua and NSE scripting, so
I would love to hear any feedback.

I haven't found the time yet to fully review the code, but at first glance it looks good.
The receive_bytes(1) could/should probably be replaced with receive().
Oh, and you probably need to fix something on line 319 :-)


Thanks
-chris
<ms-sql-info.nse>_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/


//Patrik
--
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/


  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]