From 753378cb38e3bbd7abba7765d4c8ed4d2af704ad Mon Sep 17 00:00:00 2001 From: Matias Fontanini Date: Sat, 10 Sep 2011 20:05:41 -0300 Subject: [PATCH] Fixed some leak or uninitialized memory usage bugs. --- include/dot11.h | 7 ++- include/ip.h | 1 + src/dhcp.cpp | 4 +- src/dot11.cpp | 22 +++++---- src/ip.cpp | 121 ++++++++++++++++++++++++++++-------------------- 5 files changed, 90 insertions(+), 65 deletions(-) diff --git a/include/dot11.h b/include/dot11.h index 36da870..6dea7f9 100644 --- a/include/dot11.h +++ b/include/dot11.h @@ -1241,8 +1241,10 @@ namespace Tins { void tim(uint8_t dtim_count, uint8_t dtim_period, uint8_t bitmap_control, uint8_t* partial_virtual_bitmap, uint8_t partial_virtual_bitmap_sz); uint32_t write_ext_header(uint8_t *buffer, uint32_t total_sz); + void copy_ext_header(const Dot11ManagementFrame *other); - + + uint32_t management_frame_size() { return sizeof(_ext_header) + (from_ds() && to_ds()) ? sizeof(_addr4) : 0; } private: ExtendedHeader _ext_header; uint8_t _addr4[6]; @@ -2619,7 +2621,8 @@ namespace Tins { uint32_t write_ext_header(uint8_t *buffer, uint32_t total_sz); void copy_ext_header(const Dot11Data *other); - + + uint32_t data_frame_size() { return sizeof(_ext_header) + (from_ds() && to_ds()) ? sizeof(_addr4) : 0; } private: ExtendedHeader _ext_header; uint8_t _addr4[6]; diff --git a/include/ip.h b/include/ip.h index b877622..154bd99 100644 --- a/include/ip.h +++ b/include/ip.h @@ -424,6 +424,7 @@ namespace Tins { void copy_fields(const IP *other); void init_ip_fields(); void write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *parent); + void cleanup(); iphdr _ip; std::vector _ip_options; diff --git a/src/dhcp.cpp b/src/dhcp.cpp index 770fd31..4abad7f 100644 --- a/src/dhcp.cpp +++ b/src/dhcp.cpp @@ -42,14 +42,14 @@ Tins::DHCP::DHCP(const uint8_t *buffer, uint32_t total_sz) : BootP(buffer, total total_sz -= BootP::header_size(); uint8_t args[2] = {0}; while(total_sz) { - for(unsigned i(0); i < 2 && args[0] != END; ++i) { + for(unsigned i(0); i < 2 && args[0] != END && args[0] != PAD; ++i) { args[i] = *(buffer++); total_sz--; if(!total_sz) throw std::runtime_error("Not enough size for a DHCP header in the buffer."); } // If the END-OF-OPTIONS was not found... - if(args[0] != END) { + if(args[0] != END && args[0] != PAD) { // Not enough size for this option if(total_sz < args[1]) throw std::runtime_error("Not enough size for a DHCP header in the buffer."); diff --git a/src/dot11.cpp b/src/dot11.cpp index 59de76a..33faaa0 100644 --- a/src/dot11.cpp +++ b/src/dot11.cpp @@ -621,7 +621,7 @@ Tins::Dot11Beacon::Dot11Beacon(const std::string& iface, } Tins::Dot11Beacon::Dot11Beacon(const uint8_t *buffer, uint32_t total_sz) : Dot11ManagementFrame(buffer, total_sz) { - uint32_t sz = Dot11ManagementFrame::header_size(); + uint32_t sz = management_frame_size(); buffer += sz; total_sz -= sz; if(total_sz < sizeof(_body)) @@ -828,7 +828,7 @@ Tins::Dot11Disassoc::Dot11Disassoc(const std::string& iface, } Tins::Dot11Disassoc::Dot11Disassoc(const uint8_t *buffer, uint32_t total_sz) { - uint32_t sz = Dot11ManagementFrame::header_size(); + uint32_t sz = management_frame_size(); buffer += sz; total_sz -= sz; if(total_sz < sizeof(_body)) @@ -877,7 +877,7 @@ Tins::Dot11AssocRequest::Dot11AssocRequest(const std::string& iface, } Tins::Dot11AssocRequest::Dot11AssocRequest(const uint8_t *buffer, uint32_t total_sz) : Dot11ManagementFrame(buffer, total_sz) { - uint32_t sz = Dot11ManagementFrame::header_size(); + uint32_t sz = management_frame_size(); buffer += sz; total_sz -= sz; if(total_sz < sizeof(_body)) @@ -954,7 +954,7 @@ Tins::Dot11AssocResponse::Dot11AssocResponse(const std::string& iface, } Tins::Dot11AssocResponse::Dot11AssocResponse(const uint8_t *buffer, uint32_t total_sz) : Dot11ManagementFrame(buffer, total_sz) { - uint32_t sz = Dot11ManagementFrame::header_size(); + uint32_t sz = management_frame_size(); buffer += sz; total_sz -= sz; if(total_sz < sizeof(_body)) @@ -1019,7 +1019,7 @@ Tins::Dot11ReAssocRequest::Dot11ReAssocRequest(const std::string& iface, } Tins::Dot11ReAssocRequest::Dot11ReAssocRequest(const uint8_t *buffer, uint32_t total_sz) : Dot11ManagementFrame(buffer, total_sz) { - uint32_t sz = Dot11ManagementFrame::header_size(); + uint32_t sz = management_frame_size(); buffer += sz; total_sz -= sz; if(total_sz < sizeof(_body)) @@ -1139,7 +1139,7 @@ Tins::Dot11ProbeResponse::Dot11ProbeResponse(const std::string& iface, } Tins::Dot11ProbeResponse::Dot11ProbeResponse(const uint8_t *buffer, uint32_t total_sz) : Dot11ManagementFrame(buffer, total_sz) { - uint32_t sz = Dot11ManagementFrame::header_size(); + uint32_t sz = management_frame_size(); buffer += sz; total_sz -= sz; if(total_sz < sizeof(_body)) @@ -1278,7 +1278,8 @@ Tins::Dot11Data::Dot11Data(const uint8_t *buffer, uint32_t total_sz) : Dot11(buf buffer += sizeof(_addr4); total_sz -= sizeof(_addr4); } - inner_pdu(new Tins::SNAP(buffer, total_sz)); + if(total_sz) + inner_pdu(new Tins::SNAP(buffer, total_sz)); } Tins::Dot11Data::Dot11Data(uint32_t iface_index, const uint8_t *dst_hw_addr, const uint8_t *src_hw_addr, PDU* child) : Dot11(iface_index, dst_hw_addr, child) { @@ -1376,11 +1377,12 @@ Tins::Dot11QoSData::Dot11QoSData(uint32_t iface_index, const uint8_t* dst_hw_add this->_qos_control = 0; } -Tins::Dot11QoSData::Dot11QoSData(const uint8_t *buffer, uint32_t total_sz) : Dot11Data(buffer, total_sz) { - uint32_t sz = Dot11Data::header_size(); +Tins::Dot11QoSData::Dot11QoSData(const uint8_t *buffer, uint32_t total_sz) : Dot11Data(buffer, std::min(data_frame_size(), total_sz)) { + uint32_t sz = data_frame_size(); buffer += sz; total_sz -= sz; - assert(total_sz >= sizeof(this->_qos_control)); + if(total_sz < sizeof(this->_qos_control)) + throw std::runtime_error("Not enough size for an IEEE 802.11 data header in the buffer."); this->_qos_control = *(uint16_t*)buffer; total_sz -= sizeof(uint16_t); buffer += sizeof(uint16_t); diff --git a/src/ip.cpp b/src/ip.cpp index e955b1a..5fb8879 100644 --- a/src/ip.cpp +++ b/src/ip.cpp @@ -61,66 +61,83 @@ Tins::IP &Tins::IP::operator= (const IP &other) { } Tins::IP::IP(const uint8_t *buffer, uint32_t total_sz) : PDU(IPPROTO_IP) { + static const char *msg("Not enough size for an IP header in the buffer."); if(total_sz < sizeof(iphdr)) - throw std::runtime_error("Not enough size for an IP header in the buffer."); + throw std::runtime_error(msg); std::memcpy(&_ip, buffer, sizeof(iphdr)); /* Options... */ /* Establish beginning and ending of the options */ const uint8_t* ptr_buffer = buffer + sizeof(iphdr); + if(total_sz < head_len() * sizeof(uint32_t)) + throw std::runtime_error(msg); buffer += head_len() * sizeof(uint32_t); this->_options_size = 0; this->_padded_options_size = head_len() * sizeof(uint32_t) - sizeof(iphdr); /* While the end of the options is not reached read an option */ - while (ptr_buffer < buffer && (*ptr_buffer != 0)) { - IpOption opt_to_add; - opt_to_add.optional_data = NULL; - opt_to_add.optional_data_size = 0; - memcpy(&opt_to_add.type, ptr_buffer, 1); - ptr_buffer++; - switch (opt_to_add.type.number) { - /* Multibyte options with length as second byte */ - case IPOPT_SEC: - case IPOPT_LSSR: - case IPOPT_TIMESTAMP: - case IPOPT_EXTSEC: - case IPOPT_RR: - case IPOPT_SID: - case IPOPT_SSRR: - case IPOPT_MTUPROBE: - case IPOPT_MTUREPLY: - case IPOPT_EIP: - case IPOPT_TR: - case IPOPT_ADDEXT: - case IPOPT_RTRALT: - case IPOPT_SDB: - case IPOPT_DPS: - case IPOPT_UMP: - case IPOPT_QS: - opt_to_add.optional_data_size = *ptr_buffer - 1; - opt_to_add.optional_data = new uint8_t[opt_to_add.optional_data_size]; - memcpy(opt_to_add.optional_data, ptr_buffer, opt_to_add.optional_data_size); - ptr_buffer += opt_to_add.optional_data_size; + try { + while (total_sz && ptr_buffer < buffer && (*ptr_buffer != 0)) { + IpOption opt_to_add; + opt_to_add.optional_data = 0; + opt_to_add.optional_data_size = 0; + memcpy(&opt_to_add.type, ptr_buffer, sizeof(uint8_t)); + ptr_buffer++; + switch (opt_to_add.type.number) { + /* Multibyte options with length as second byte */ + case IPOPT_SEC: + case IPOPT_LSSR: + case IPOPT_TIMESTAMP: + case IPOPT_EXTSEC: + case IPOPT_RR: + case IPOPT_SID: + case IPOPT_SSRR: + case IPOPT_MTUPROBE: + case IPOPT_MTUREPLY: + case IPOPT_EIP: + case IPOPT_TR: + case IPOPT_ADDEXT: + case IPOPT_RTRALT: + case IPOPT_SDB: + case IPOPT_DPS: + case IPOPT_UMP: + case IPOPT_QS: + if(!total_sz || *ptr_buffer == 0) + throw std::runtime_error(msg); + opt_to_add.optional_data_size = *ptr_buffer - 1; + if(opt_to_add.optional_data_size > 0) { + if(total_sz < opt_to_add.optional_data_size) + throw std::runtime_error(msg); + opt_to_add.optional_data = new uint8_t[opt_to_add.optional_data_size]; + memcpy(opt_to_add.optional_data, ptr_buffer, opt_to_add.optional_data_size); + } + else + opt_to_add.optional_data = 0; + ptr_buffer += opt_to_add.optional_data_size; + } + this->_ip_options.push_back(opt_to_add); + this->_options_size += 1 + opt_to_add.optional_data_size; + } + total_sz -= head_len() * sizeof(uint32_t); + if (total_sz) { + switch(_ip.protocol) { + case IPPROTO_TCP: + inner_pdu(new Tins::TCP(buffer, total_sz)); + break; + case IPPROTO_UDP: + inner_pdu(new Tins::UDP(buffer, total_sz)); + break; + case IPPROTO_ICMP: + inner_pdu(new Tins::ICMP(buffer, total_sz)); + break; + default: + inner_pdu(new Tins::RawPDU(buffer, total_sz)); + break; + } } - this->_ip_options.push_back(opt_to_add); - this->_options_size += 1 + opt_to_add.optional_data_size; } - total_sz -= head_len() * sizeof(uint32_t); - if (total_sz) { - switch(_ip.protocol) { - case IPPROTO_TCP: - inner_pdu(new Tins::TCP(buffer, total_sz)); - break; - case IPPROTO_UDP: - inner_pdu(new Tins::UDP(buffer, total_sz)); - break; - case IPPROTO_ICMP: - inner_pdu(new Tins::ICMP(buffer, total_sz)); - break; - default: - inner_pdu(new Tins::RawPDU(buffer, total_sz)); - break; - } + catch(runtime_error &) { + cleanup(); + throw; } } @@ -136,9 +153,12 @@ Tins::IP::IP(uint32_t ip_dst, uint32_t ip_src, PDU *child) : PDU(IPPROTO_IP, chi } Tins::IP::~IP() { + cleanup(); +} + +void Tins::IP::cleanup() { for (vector::iterator it = this->_ip_options.begin(); it != this->_ip_options.end(); it++) { - if (it->optional_data) - delete[] it->optional_data; + delete[] it->optional_data; } } @@ -237,7 +257,6 @@ void Tins::IP::set_option(uint8_t copied, } uint8_t* Tins::IP::IpOption::write(uint8_t* buffer) { - memcpy(buffer, &type, 1); buffer += 1; if (optional_data) {