Re: [NSE] Improved version of ms-sql-info

[Chris Woodbury <chris3e3 () gmail com>]

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/

Attachment: mssql.lua
Description:

Attachment: mssql_refactored.patch
Description:

Attachment: ms-sql-info_refactored.nse
Description:

Attachment: ms-sql-info_standalone.nse
Description:

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