Re: [NSE] smb-ls fixes and improvements

[Henri Doreau <henri.doreau () gmail com>]

2015-03-07 20:36 GMT+01:00 Pierre LALET <pierre () droids-corp org>:
> Hi list,
>
> Here is a patch that:
>
>   - Fixes a bug in smb.lua (smb-ls was broken, at least against a
>     Samba service I found).
>
>   - Allows smb-ls to be used with multiple shares at once and creates
>     a default value ("\") for its `path` argument.
>
>   - Allows smb-enum-shares to tell smb-ls which share to browse when
>     no share has been specified.
>
> I'm new to LUA & NSE so forgive me if my code is not as good as it
> should be.
>
> I have tried to make sure that the script smb-ls and smb-enum-shares
> still work as before when used with the same arguments (no API was
> harmed in the making of this patch).
>
> I think we should consider to use 1 as default value when no value has
> been set for `maxdepth` parameter (for now it defaults to 0 which can
> take a *really* long time to complete on some shares, 1 would mean "no
> recursion").
>
> I did not add that in this patch because that would change the current
> behavior of smb-ls.
>
> Of course, remarks and comments welcome!
>
> --
> Pierre
> http://pierre.droids-corp.org/

Hi Pierre,

thanks for the patch. It looks very good. A couple minor comments though:

* improvement: Would it make sense to identify the service port on
which share was found in the host registry? So that scripts can
efficiently interact with hosts running multiple SMB instances?

* style: Limit depth of nested blocks. At the start of the action()
function of smb-ls, you can flatten the structure by doing:
"""
 -- give priority to specified shares if specified
  if arg_shares ~= nil then
     arg_shares = stdnse.strsplit(",", arg_shares)
  elseif arg_share ~= nil then
     arg_shares = {arg_share}
  else
     arg_shares = host.registry['smb_shares']
  end
"""

* It was already in the script, but this lua idiom either needs a
rewrite or a comment! :)
"""
  local lstab = tab.new((arg_checksum and 4 or 3))
"""

As for the behavior regarding recursion I would be ok with the default
value from zero to one, unless others have different opinion on the
matter. Anyone?


Regards

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