Nmap Development mailing list archives
Re: [NSE] Schneider NSE(modicon-info.nse)
From: Daniel Miller <bonsaiviking () gmail com>
Date: Mon, 23 Feb 2015 22:47:59 -0600
Stephen, Thanks for the contribution! It's always exciting to see these ICS/SCADA devices laid bare with a little ingenuity. I have a few suggestions for improvements that I'd like to see before we integrate this: 1. Use the shortport.port_or_service portrule with the "modbus" service name. Version detection has a few matches for modbus, so we should be flexible for strange cases (port forwarding, maybe?) where Modbus runs on a port other than 502. 2. Provide a bit more detail on the values in packets you are sending. Much like how I dissected the BACnet probes in bacnet-info recently, it's better to use other format operators to bin.pack than "H" so that we can document exactly what is being sent. 3. It would be nice if there were some way to make sure the device is still responding properly during all those send-receives in init_comms. We should at least check the return status for socket:receive(), but it would be nice to be able to check the data, too. 4. To build a string of repeated characters, use string.rep() instead of looping over a concatenation operator. This is not only cleaner, but more efficient. Related, if you build a string (project_info) by looping over concatenation, try using the strbuf library instead. 5. Chained calls to bin.unpack() where the previous pos return value is passed directly to the next call can be rewritten into one call. So this: pos, project_rev_1 = bin.unpack("C", response, pos ) pos, project_rev_2 = bin.unpack("C", response, pos) pos, project_rev_3 = bin.unpack("C", response, pos) Is equivalent to this: pos, project_rev_1, project_rev_2, project_rev_3 = bin.unpack("C3", response, pos) 6. Try the length-prefixed string format operators to bin.unpack. So this: pos, size = bin.unpack("C", response, pos + 1) pos, output["Network Module"] = bin.unpack("A" .. size, response, pos) Becomes this: pos, output["Network Module"] = bin.unpack("p", response, pos + 1) 7. You can test if a string is == "" instead of checking if the length is 0, it's a bit cleaner. 8. Use the shell script or the git hooks on our Code Standards wiki page to help find problems with global variables: https://secwiki.org/w/Nmap/Code_Standards#Tools_to_help Sorry for the laundry list, but I thought some of these tips might be helpful for other script authors, too. I'll look at the other scripts you've submitted recently and let you know if there's anything else we'd need before including those, so watch for those messages. Dan On Tue, Sep 30, 2014 at 3:15 PM, Stephen Hilt <hilt () digitalbond com> wrote:
Hello, This weekend I gave a talk at DerbyCon and released a new NSE for Schneider Electric Modicon Devices. The script can be found https://github.com/digitalbond/Redpoint/blob/master/modicon-info.nse For More information, head on over to our blog and read the post we made about the script. http://www.digitalbond.com/blog/2014/09/29/redpoint-schneidermodicon-plc-enumeration/ _______________________________________________ Sent through the dev mailing list http://nmap.org/mailman/listinfo/dev Archived at http://seclists.org/nmap-dev/
_______________________________________________ Sent through the dev mailing list https://nmap.org/mailman/listinfo/dev Archived at http://seclists.org/nmap-dev/
Current thread:
- Re: [NSE] Schneider NSE(modicon-info.nse) Daniel Miller (Feb 23)
- Re: [NSE] Schneider NSE(modicon-info.nse) Stephen Hilt (Mar 07)