Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: [NSE] Improved version of ms-sql-info
From: Chris Woodbury <chris3e3 () gmail com>
Date: Fri, 28 Jan 2011 12:47:34 -0600

I just realized that I added a bug in existing mssql.lua code (in
Helper.Discover()) by adding a debugging message that isn't needed
anyway. This should revert it:

Index: mssql.lua
===================================================================
--- mssql.lua   (working copy)
+++ mssql.lua   (working copy)
@@ -1240,11 +1240,7 @@
                
                repeat
                        status, data = socket:receive()
-                       if ( not(status) ) then
-                               stdnse.print_debug( 2, "%s: Socket:receive() error: %s",
"mssql.lua", data )
-                               socket:close()
-                               return status, data
-                       end
+                       if ( not(status) ) then break end

                        -- strip of first 3 bytes as they contain thing we don't want
                        data = data:sub(4)


On Fri, Jan 28, 2011 at 3:25 AM, Chris Woodbury <chris3e3 () gmail com> wrote:
Patrik-

Thanks for the suggestions. I went ahead and built a class that fully
implements the PreLogin packet (both client->server and
server->client). It's much more robust on its own, and I also added
some more error-handling and rigor throughout.

I have two different flavors, depending on what's easier for you (and
others) to review and test out:
1) A stand-alone version of ms-sql-info.nse [1], which should work
with the existing mssql.lua.
2) Updated versions of ms-sql-info.nse [2] and mssql.lua [3][4], with
much of the code factored into mssql.lua.

Functionally, they are the same; the only differences are where the code is.

Let me know what you think.
Thanks
-chris

Attached:
[1] ms-sql-info_standalone.nse
[2] ms-sql-info_refactored.nse
[3] mssql.lua
[4] mssql_refactored.patch ([3] as a patch against r21987)

On Wed, Jan 26, 2011 at 12:07 PM, Patrik Karlsson <patrik () cqure net> wrote:
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/


_______________________________________________
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 ]
AlienVault