Home page logo

nmap-dev logo Nmap Development mailing list archives

Re: Several SNMP script additions
From: Patrik Karlsson <patrik () cqure net>
Date: Sun, 25 Dec 2011 22:15:11 +0100

On Sat, Dec 24, 2011 at 3:19 AM, Brendan Byrd <sineswiper () gmail com> wrote:

On Sat, Dec 17, 2011 at 8:55 PM, Brendan Byrd <sineswiper () gmail com>

Got a bunch of library and script changes.  Here's the list of changes:

Anybody willing to put in this into the SVN, or at least discuss the code?
I know it's X-Mas season, but I figured I'd get a response by now.

Brendan Byrd/SineSwiper <SineSwiper () GMail com>
Sent through the nmap-dev mailing list
Archived at http://seclists.org/nmap-dev/

Hi Brendan,

First and foremost, thanks for your contribution! I've spent quite some time
going through it, and there's some good things in there. The snmp-system
looks really interesting and I agree that it could be a candidate for
the snmp-sysdescr. I've tested it and added a fingerprint for my Mac Os X
system. While doing so, I found a bug that would prevent the script from
completing. Also, I think the extrainfo should probably be left out as
there is
a lot of redundant info in there. Also in my case, this section is truncated
and barely readable. I've attached a patch to make these changes.

On that same subject I would personally appreciate if you could send the
set of changes you make as patches against subversion. This makes it a lot
easier to review. Also, I noticed that there were quite a few changes in
whitespace, line feeds and minor modifications to things like string matches
and length checks, eg:
* string length checks have been changed from #str to string.len(str)
* string matching has been changed from str:match to string.match (or even
  match in ipOps)

Initially I thought they were all your changes but I'm no longer sure as I
noticed that some of the files may not have been the current versions?
while I guess those changes are probably OK, it would be a lot easier to
them in separate code style patches. Also, if you do changes to code style
sure they are consistent across the whole script or library.

Please find comments for each of the files in the zip-archive below:
* asn1.lua
  - This file looks unchanged to me, is that correct?

* ipOps.lua
  - The CIDR additions look very interesting though I haven't been able to
    any testing of them yet. In regards to moving them to the C, I'm
    not the right person to comment on that.

* snmp-brute.nse
  - It looks as if most changes make sense, unfortunately Duarte Silva
    working on some changes to the script at the same time as you. He posted
    an update while is was working on your answer, and I haven't had a
    to look at it yet. One thing, in addition to the changes you've already
    made, there is a problem with the script silently failing when it can't
    load a community list. Even though the debug message is there, it should
    really return an error instead of nothing.

* snmp-routing.nse
  - This script looks impressive, what devices will it work with? I'm not
    I have anything at hand to test it against. If you let me/the list know
    what's needed maybe someone can give it a go?

* snmp-system.nse
  - Like I mentioned, this script looks really nice and I've added a
    fingerprint for Darwin, which in my case is really OS X. I guess this
    call for some lookup table for OS:es, so that things get translated
    properly. Also, like I already mentioned, I think the extrainfo should
    left out.

* snmp.lua
  - I've merged some of the changes and left the ones that I wasn't sure of.
    There are a few if statements that were changed, that no longer reflect
    what they used to and I'm not sure that all of those are actually
    Also, I'm not sure whether it's best to return a partial "half-broken"
    result, instead of an error indicating a timeout which would allow a
    script to retry the query. As the function buildGetBulkRequest was not
    working, I left it out. I've included a patch that shows the diff
    your version and what's currently in subversion.

* stdnse.lua
  - I don't know if you made any changes to this library, but I'm guessing
    if you did, they were made against an old version. There is a function
    missing and also some functions look different now which will break some

* target.lua
  - At first I didn't understand the problem with duplicates but after
    through the code and doing some testing I do. The problem occurs when
    adding ranges and I think your analysis is probably right, this should
    probably be handled elsewhere and not by the library.

In regards to duplicate IP's and ranges and such there's really some
discrepancies as you can add duplicate ip's or overlapping ranges on both
command line or in an input list using -iL but the target library attempts
prevent it. So maybe this is more of a Nmap question than a library
I mean should you be able to add duplicate ip's and ranges and if so why
should they be treated any different if added through the target library?

So to summarize:
* Great work!
* Could you please submit your changes as diffs and make sure they're
  what's in subversion.
* Let's see if we can get some clarity in the duplicates issues and what
  people think.


Patrik Karlsson

Attachment: snmp.diff

Attachment: snmp-system.diff

Sent through the nmap-dev mailing list
Archived at http://seclists.org/nmap-dev/

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]