From dd40ba14eac27fcadc022aecded48e7b133b81c1 Mon Sep 17 00:00:00 2001 From: dmiller Date: Tue, 10 Feb 2015 04:49:28 +0000 Subject: [PATCH] Remove data packing atrocities from broadcast-ping.nse Use of the "H" bin.unpack template should be discouraged, since it leads to use of blobs of hex data without dissection. NSE scripts should be self-documenting with regard to packet contents. Similarly, chaining bin.pack and bin.unpack is usually an anti-pattern for some simpler construct. In this case, converting a number to hex, padding it with "0", and packing it is unnecessary, since the original number can be packed directly with the proper endianness and width. --- scripts/broadcast-ping.nse | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/scripts/broadcast-ping.nse b/scripts/broadcast-ping.nse index ac7990ccb..e5fe8215b 100644 --- a/scripts/broadcast-ping.nse +++ b/scripts/broadcast-ping.nse @@ -99,31 +99,26 @@ local icmp_packet = function(srcIP, dstIP, ttl, data_length, mtu, seqNo, icmp_id icmp_payload = "" end - local seqNo_hex = stdnse.tohex(seqNo) - local icmp_seqNo = bin.pack(">H", string.rep("0",(4-seqNo_hex))..seqNo_hex) - -- Type=08; Code=00; Chksum=0000; ID=icmp_id; SeqNo=icmp_seqNo; Payload=icmp_payload(hex string); - local icmp_tmp = bin.pack(">HAAA", "0800 0000", icmp_id, icmp_seqNo, icmp_payload) + local icmp_msg = bin.pack(">CCSASA", 8, 0, 0, icmp_id, seqNo, icmp_payload) - local icmp_checksum = packet.in_cksum(icmp_tmp) + local icmp_checksum = packet.in_cksum(icmp_msg) - local icmp_msg = bin.pack(">HHAAA", "0800", stdnse.tohex(icmp_checksum), icmp_id, icmp_seqNo, icmp_payload) + icmp_msg = bin.pack(">CCSASA", 8, 0, icmp_checksum, icmp_id, seqNo, icmp_payload) - --IP Total Length - local length_hex = stdnse.tohex(20 + #icmp_msg) - local ip_length = bin.pack(">H", string.rep("0",(4-#length_hex))..length_hex) - - --TTL - local ttl_hex = stdnse.tohex(ttl) - local ip_ttl = bin.pack(">H", string.rep("0",(2-ttl_hex))..ttl_hex) - --IP header - local ip_bin = bin.pack(">HAHAH","4500",ip_length, "0000 4000", ip_ttl, - "01 0000 0000 0000 0000 0000") + local ip_bin = bin.pack(">ASSACCx10", -- x10 = checksum & addresses + "\x45\x00", -- IPv4, no options, no DSCN, no ECN + 20 + #icmp_msg, -- total length + 0, -- IP ID + "\x40\x00", -- DF + ttl, + 1 -- ICMP + ) -- IP+ICMP; Addresses and checksum need to be filled - local icmp_bin = bin.pack(">AA",ip_bin, icmp_msg) + local icmp_bin = ip_bin .. icmp_msg --Packet local icmp = packet.Packet:new(icmp_bin,#icmp_bin) @@ -176,7 +171,12 @@ local broadcast_if = function(if_table,icmp_responders) local icmp = icmp_packet( source_IP, destination_IP, ttl, data_length, mtu, sequence_number, icmp_id) - local ethernet_icmp = bin.pack("HAHA", "FF FF FF FF FF FF", if_table.mac, "08 00", icmp.buf) + local ethernet_icmp = ( + "\xFF\xFF\xFF\xFF\xFF\xFF" -- dst mac + .. if_table.mac -- src mac + .. "\x08\x00" -- ethertype IPv4 + .. icmp.buf -- data + ) try( dnet:ethernet_send(ethernet_icmp) ) end