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

[Patrik Karlsson <patrik () cqure net>]

On 31 jan 2011, at 22.08, Chris Woodbury wrote:

> On Sun, Jan 30, 2011 at 4:04 AM, Patrik Karlsson <patrik () cqure net> wrote:
> > Although I'm no Lua expert here are some quick comments on the code:
> > * You should probably avoid the leading underscore convention for instance variables and function names to avoid 
> > confusion with metamethods. [1]
>
> Good tip, and thanks for the reference. I've replaced the leading
> double-underscores with single underscores. Do you think that's still
> likely to cause problems? My background is in object-oriented
> programming, so I'm wondering whether I'm trying to impose too much on
> Lua. :)

I don't think it will cause any problems but theres not much NSE code currently that uses this convention ....
On the other hand, theres no code style guide for NSE saying the opposite either.

> >   On that same note, all of my ToString and your new ToBytes method could be changed to __tostring (as I've done in 
> > my later libraries).
> >   This would make it possible to do:
> >
> > local p = PreLoginPacket:new()
> > socket:send( tostring(p) )
>
> I did this and then realized that the ToString methods here return two
> values (PacketType, PacketData). I didn't think it was worth it to
> make all the necessary changes to make that work, so I reverted back
> to the original. I'll keep that in mind for the future, though.

Ok, sorry, my bad.

> > * I don't know if this is or is going to be a problem but os.time() returns a value in seconds. So if two 
> > PreLoginPackets are created within the same second, the thread id will be the same.
> >   In the Connect method I'm adding the local port of the socket into the randomseed together with os.time. Looking 
> > at that particular code now, it could probably be cleaned up a little :)
>
> According to the TDS spec, ThreadID is just for debugging (i.e.
> tracing) purposes. I think the server pretty much ignores it - in
> fact, the server doesn't even return the value (or any value) in the
> response. So, I think it's safe to leave things as they are. That
> said, I realized that my random seeding was unnecessary anyway, since
> it's already being done in TDSStream.Connect().

Ok, great.

> Attached is an updated patch with the changes you've suggested. The
> only other new thing is that I've factored out most of the logic from
> the Helper.Discover() into a new SSRP class, which clarifies the
> process a bit and does some extra error-checking.

I was just doing some testing so that I could commit this code but noticed something strange.
I haven't investigated it further but the new ms-sql-info script does not report the TCP ports for my instances.
This is what I'm seeing:

PORT     STATE SERVICE  REASON
1434/udp open  ms-sql-m script-set
| ms-sql-info: 
|   Server name: EDUSRV011
|     Instance name: SQLEXPRESS
|       Version: Microsoft SQL Server 2005 SP2
|         Product: Microsoft SQL Server 2005
|         Service pack level: SP2
|       Clustered: No
|     Instance name: MSSQLSERVER
|       Version: Microsoft SQL Server 2000
|         Product: Microsoft SQL Server 2000
|       Named pipe: \\EDUSRV011\pipe\sql\query
|_      Clustered: No


This is how the SSRP string looks:
ServerName;EDUSRV011;InstanceName;MSSQLSERVER;IsClustered;No;Version;8.00.194;np;\\EDUSRV011\pipe\sql\query;tcp;1433;;ServerName;EDUSRV011;InstanceName;SQLEXPRESS;IsClustered;No;Version;9.00.3042.00;tcp;1444;via;EDUSRV011,0:1433;;

Could you please look in to why this information does not make it to the output? If you need more debug info, let me 
know.

> Thanks
> -chris
> <mssql.patch>


Regards,
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/