Re: indexing globals in msrpc, msrpctypes and smb libs

[Ron <ron () skullsecurity net>]

jah wrote:
> Hi Ron,
>
> I ran Patrick's check_globals script [1] which reported that there were
> a few globals indexed in a few NSE libraries:
>
> Checking nselib/msrpc.lua for bad global accesses
>         Found indexed global,'server_unc', at line number 0.
>
> Checking nselib/msrpctypes.lua for bad global accesses
>         Found indexed global,'count', at line number 0.
>         Found indexed global,'marshal_int16', at line number 0.
>         Found indexed global,'marshal_int8', at line number 0.
>         Found indexed global,'marshall_password', at line number 0.
>         Found indexed global,'svcctl_Type_str', at line number 0.
>         Found indexed global,'svcctl_State_str', at line number 0.
>
> Checking nselib/smb.lua for bad global accesses
>         Found indexed global,'use_defaults', at line number 0.
>         Found indexed global,'count_reserved', at line number 0.
>
> Attached is a patch which hopefully fixes some of these - perhaps you'll
> look it over to make sure I've not done something stupid, particularly
> the 'server_unc' on line 663 of msrpc.lua and 'count_reserved' on lines
> starting at 1792 of smb.lua.
The whole patch looks good to me -- the issues appear to all be
copy/paste errors on my part. Thanks for fixing them!

> That leaves the following issues remaining:
>
> Checking nselib/msrpctypes.lua for bad global accesses
>         Found indexed global,'marshall_password', at line number 2922.
>         Found indexed global,'svcctl_Type_str', at line number 4263.
>         Found indexed global,'svcctl_State_str', at line number 4317.
>
> Those functions currently don't exist.
> I note that marshall_password() is not needed for windows.  It is passed
> to marshall_ptr() in marshall_srvsvc_NetShareInfo2() which is a code
> path I haven't been able to execute so far.
That codepath appears to be unused, which is likely why it hasn't caused
an issue yet. It should have been marshall_unicode(), I changed it.

> svcctl_Type_str() and svcctl_State_str() are called from
> svcctl_Type_tostr() and svcctl_State_tostr() respectively - neither of
> which are called by any script or any library.
> Do you have any suggestions for dealing with these?
I occasionally implemented structs that I don't actually use, but that
might be used after. If they're causing a warning, they can likely be
commented out. Otherwise, I'd leave them.

> On a related note, the attached patch for smb-brute.nse fixes a global
> access revealed using strict.lua:
>
> ...smb-brute.nse:178: variable 'random_set' is not declared
>
> random_set is set true once math.random() has been seeded and the patch
> uses nmap.registry to achieve the same.  Does that look OK to you?
Yep, looks good to me!

> Regards,
>
> jah
>
> [1] - http://seclists.org/nmap-dev/2009/q3/0070.html


Thanks a bunch for doing this! Running my code through the
global-variable checker has been on my TODO list for awhile, I'm glad
you took care of it. Are there any issues/files you haven't checked that
I'll need to keep an eye on?

Thanks!
Ron

-- 
Ron Bowes
http://www.skullsecurity.org/

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