From 20054e6c73c1003829169e8595d322998db4e0c2 Mon Sep 17 00:00:00 2001 From: Matias Fontanini Date: Mon, 8 Apr 2013 11:58:12 -0300 Subject: [PATCH] Several classes now use PDUOption::length_field instead of data_size. --- include/dhcpv6.h | 74 +------------------------------------------- include/pdu_option.h | 7 ++++- src/dhcp.cpp | 2 +- src/dhcpv6.cpp | 2 +- src/dot11.cpp | 2 +- src/dot1q.cpp | 4 ++- src/icmpv6.cpp | 2 +- src/ip.cpp | 2 +- src/ipv6.cpp | 2 +- src/tcp.cpp | 4 +-- tests/src/icmpv6.cpp | 17 ++++++++++ tests/src/ip.cpp | 17 ++++++++++ tests/src/pppoe.cpp | 1 + tests/src/tcp.cpp | 17 ++++++++++ 14 files changed, 70 insertions(+), 83 deletions(-) diff --git a/include/dhcpv6.h b/include/dhcpv6.h index 89ccf6a..06e4cb4 100644 --- a/include/dhcpv6.h +++ b/include/dhcpv6.h @@ -46,79 +46,7 @@ public: /** * Represents a DHCPv6 option. */ - class dhcpv6_option { - public: - typedef std::vector container_type; - typedef container_type::value_type data_type; - typedef uint16_t option_type; - - /** - * \brief Constructs a PDUOption. - * \param opt The option type. - * \param length The option's data length. - * \param data The option's data(if any). - */ - dhcpv6_option(option_type opt = 0, size_t length = 0, const data_type *data = 0) - : option_(opt), option_size_(length) { - if(data) - value_.insert(value_.end(), data, data + length); - } - - /** - * \brief Constructs a PDUOption from iterators, which - * indicate the data to be stored in it. - * - * \param opt The option type. - * \param start The beginning of the option data. - * \param end The end of the option data. - */ - template - dhcpv6_option(option_type opt, ForwardIterator start, ForwardIterator end) - : option_(opt), option_size_(std::distance(start, end)) - { - value_.insert(value_.end(), start, end); - } - - /** - * Retrieves this option's type. - * \return uint8_t containing this option's size. - */ - uint16_t option() const { - return option_; - } - - /** - * Sets this option's type - * \param opt The option type to be set. - */ - void option(uint16_t opt) { - option_ = opt; - } - - /** - * Retrieves this option's data. - * - * If this method is called when data_size() == 0, - * dereferencing the returned pointer will result in undefined - * behaviour. - * - * \return const data_type& containing this option's value. - */ - const data_type *data_ptr() const { - return &*value_.begin(); - } - - /** - * Retrieves the length of this option's data. - */ - uint16_t data_size() const { - return option_size_; - } - private: - option_type option_; - uint16_t option_size_; - container_type value_; - }; + typedef PDUOption dhcpv6_option; /** * The message types. diff --git a/include/pdu_option.h b/include/pdu_option.h index 45a6f45..10a0679 100644 --- a/include/pdu_option.h +++ b/include/pdu_option.h @@ -153,7 +153,12 @@ public: /** * \brief Retrieves the data length field. * - * This may be different to the actual size of the data. + * This may be different to the actual size of the data. Note that + * in some protocols, such as TCP, the size of the length and the + * identifier fields is added to this field before serializing. + * Therefore, if on one of such protocols, an option's length_field + * returns X, then the actual length included in the serialized + * option will be X + C, where C is the size of those fields. * * \sa data_size. */ diff --git a/src/dhcp.cpp b/src/dhcp.cpp index 3521f73..c6bc8ae 100644 --- a/src/dhcp.cpp +++ b/src/dhcp.cpp @@ -222,7 +222,7 @@ void DHCP::write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *pa *((uint32_t*)&result[0]) = Endian::host_to_be(0x63825363); for(options_type::const_iterator it = _options.begin(); it != _options.end(); ++it) { *(ptr++) = it->option(); - *(ptr++) = it->data_size(); + *(ptr++) = it->length_field(); std::copy(it->data_ptr(), it->data_ptr() + it->data_size(), ptr); ptr += it->data_size(); } diff --git a/src/dhcpv6.cpp b/src/dhcpv6.cpp index 1b768a1..b398d6e 100644 --- a/src/dhcpv6.cpp +++ b/src/dhcpv6.cpp @@ -93,7 +93,7 @@ const DHCPv6::dhcpv6_option *DHCPv6::search_option(Option id) const { uint8_t* DHCPv6::write_option(const dhcpv6_option &option, uint8_t* buffer) const { *(uint16_t*)buffer = Endian::host_to_be(option.option()); - *(uint16_t*)&buffer[sizeof(uint16_t)] = Endian::host_to_be(option.data_size()); + *(uint16_t*)&buffer[sizeof(uint16_t)] = Endian::host_to_be(option.length_field()); return std::copy( option.data_ptr(), option.data_ptr() + option.data_size(), diff --git a/src/dot11.cpp b/src/dot11.cpp index 3d0d1f3..865d5a6 100644 --- a/src/dot11.cpp +++ b/src/dot11.cpp @@ -221,7 +221,7 @@ void Dot11::write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *p assert(total_sz >= child_len + _options_size); for(std::list::const_iterator it = _options.begin(); it != _options.end(); ++it) { *(buffer++) = it->option(); - *(buffer++) = it->data_size(); + *(buffer++) = it->length_field(); std::copy(it->data_ptr(), it->data_ptr() + it->data_size(), buffer); buffer += it->data_size(); } diff --git a/src/dot1q.cpp b/src/dot1q.cpp index e344e8b..34945fd 100644 --- a/src/dot1q.cpp +++ b/src/dot1q.cpp @@ -41,7 +41,9 @@ Dot1Q::Dot1Q(small_uint<12> tag_id, bool append_pad) id(tag_id); } -Dot1Q::Dot1Q(const uint8_t *buffer, uint32_t total_sz) { +Dot1Q::Dot1Q(const uint8_t *buffer, uint32_t total_sz) +: _append_padding() +{ if(total_sz < sizeof(_header)) throw std::runtime_error("Not enough size for a Dot1Q header"); std::memcpy(&_header, buffer, sizeof(_header)); diff --git a/src/icmpv6.cpp b/src/icmpv6.cpp index ca62bbc..99a3766 100644 --- a/src/icmpv6.cpp +++ b/src/icmpv6.cpp @@ -254,7 +254,7 @@ void ICMPv6::internal_add_option(const icmpv6_option &option) { uint8_t *ICMPv6::write_option(const icmpv6_option &opt, uint8_t *buffer) { *buffer++ = opt.option(); - *buffer++ = (opt.data_size() + sizeof(uint8_t) * 2) / 8; + *buffer++ = (opt.length_field() + sizeof(uint8_t) * 2) / 8; return std::copy(opt.data_ptr(), opt.data_ptr() + opt.data_size(), buffer); } diff --git a/src/ip.cpp b/src/ip.cpp index a2f606d..432ac1f 100644 --- a/src/ip.cpp +++ b/src/ip.cpp @@ -321,7 +321,7 @@ uint8_t* IP::write_option(const ip_option &opt, uint8_t* buffer) { option_identifier opt_type = opt.option(); memcpy(buffer, &opt_type, 1); buffer++; - *(buffer++) = opt.data_size() + 2; + *(buffer++) = opt.length_field() + 2; std::copy(opt.data_ptr(), opt.data_ptr() + opt.data_size(), buffer); buffer += opt.data_size(); return buffer; diff --git a/src/ipv6.cpp b/src/ipv6.cpp index d959426..9fa4267 100644 --- a/src/ipv6.cpp +++ b/src/ipv6.cpp @@ -264,7 +264,7 @@ void IPv6::set_last_next_header(uint8_t value) { uint8_t *IPv6::write_header(const ipv6_ext_header &header, uint8_t *buffer) { *buffer++ = header.option(); - *buffer++ = (header.data_size() > 8) ? (header.data_size() - 8) : 0; + *buffer++ = (header.length_field() > 8) ? (header.length_field() - 8) : 0; return std::copy(header.data_ptr(), header.data_ptr() + header.data_size(), buffer); } diff --git a/src/tcp.cpp b/src/tcp.cpp index 9f15258..ccc0f52 100644 --- a/src/tcp.cpp +++ b/src/tcp.cpp @@ -346,10 +346,10 @@ uint8_t *TCP::write_option(const tcp_option &opt, uint8_t *buffer) { } else { buffer[0] = opt.option(); - buffer[1] = opt.data_size() + (sizeof(uint8_t) << 1); + buffer[1] = opt.length_field() + (sizeof(uint8_t) << 1); if(opt.data_size() != 0) std::copy(opt.data_ptr(), opt.data_ptr() + opt.data_size(), buffer + 2); - return buffer + buffer[1]; + return buffer + opt.data_size() + (sizeof(uint8_t) << 1); } } diff --git a/tests/src/icmpv6.cpp b/tests/src/icmpv6.cpp index 1faea65..adc327a 100644 --- a/tests/src/icmpv6.cpp +++ b/tests/src/icmpv6.cpp @@ -417,3 +417,20 @@ TEST_F(ICMPv6Test, DNSSearchList) { EXPECT_EQ(output.lifetime, data.lifetime); EXPECT_EQ(data.domains, output.domains); } + +TEST_F(ICMPv6Test, SpoofedOptions) { + ICMPv6 pdu; + uint8_t a[] = { 1,2,3,4,5,6 }; + pdu.add_option( + ICMPv6::icmpv6_option(ICMPv6::NAACK, 250, a, a + sizeof(a)) + ); + pdu.add_option( + ICMPv6::icmpv6_option(ICMPv6::NAACK, 250, a, a + sizeof(a)) + ); + pdu.add_option( + ICMPv6::icmpv6_option(ICMPv6::NAACK, 250, a, a + sizeof(a)) + ); + // probably we'd expect it to crash if it's not working, valgrind plx + EXPECT_EQ(3, pdu.options().size()); + EXPECT_EQ(pdu.serialize().size(), pdu.size()); +} diff --git a/tests/src/ip.cpp b/tests/src/ip.cpp index 4e4afb8..b7b9d19 100644 --- a/tests/src/ip.cpp +++ b/tests/src/ip.cpp @@ -253,3 +253,20 @@ TEST_F(IPTest, StackedProtocols) { buffer = ip.serialize(); EXPECT_TRUE(IP(&buffer[0], buffer.size()).find_pdu()); } + +TEST_F(IPTest, SpoofedOptions) { + IP pdu; + uint8_t a[] = { 1,2,3,4,5,6 }; + pdu.add_option( + IP::ip_option(IP::NOOP, 250, a, a + sizeof(a)) + ); + pdu.add_option( + IP::ip_option(IP::NOOP, 250, a, a + sizeof(a)) + ); + pdu.add_option( + IP::ip_option(IP::NOOP, 250, a, a + sizeof(a)) + ); + // probably we'd expect it to crash if it's not working, valgrind plx + EXPECT_EQ(3, pdu.options().size()); + EXPECT_EQ(pdu.serialize().size(), pdu.size()); +} diff --git a/tests/src/pppoe.cpp b/tests/src/pppoe.cpp index 1ba7c89..2e4061d 100644 --- a/tests/src/pppoe.cpp +++ b/tests/src/pppoe.cpp @@ -173,4 +173,5 @@ TEST_F(PPPoETest, SpoofedOptions) { ); // probably we'd expect it to crash if it's not working, valgrind plx EXPECT_EQ(3, pdu.tags().size()); + EXPECT_EQ(pdu.serialize().size(), pdu.size()); } diff --git a/tests/src/tcp.cpp b/tests/src/tcp.cpp index 060a263..4413364 100644 --- a/tests/src/tcp.cpp +++ b/tests/src/tcp.cpp @@ -210,3 +210,20 @@ TEST_F(TCPTest, Serialize) { ASSERT_EQ(buffer.size(), sizeof(expected_packet)); EXPECT_TRUE(std::equal(buffer.begin(), buffer.end(), expected_packet)); } + +TEST_F(TCPTest, SpoofedOptions) { + TCP pdu; + uint8_t a[] = { 1,2,3,4,5,6 }; + pdu.add_option( + TCP::tcp_option(TCP::SACK, 250, a, a + sizeof(a)) + ); + pdu.add_option( + TCP::tcp_option(TCP::SACK, 250, a, a + sizeof(a)) + ); + pdu.add_option( + TCP::tcp_option(TCP::SACK, 250, a, a + sizeof(a)) + ); + // probably we'd expect it to crash if it's not working, valgrind plx + EXPECT_EQ(3, pdu.options().size()); + EXPECT_EQ(pdu.serialize().size(), pdu.size()); +}