From c20c82bcb5521c6050580523d31f56a9ef361f56 Mon Sep 17 00:00:00 2001 From: Gaya Cohen Date: Mon, 24 May 2021 15:12:23 +0300 Subject: [PATCH 1/4] Fix pointer loop bug and add descriptive exceptions --- include/tins/exceptions.h | 13 +++++++++++++ src/dns.cpp | 6 +++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/include/tins/exceptions.h b/include/tins/exceptions.h index 844439b..3ef91ba 100644 --- a/include/tins/exceptions.h +++ b/include/tins/exceptions.h @@ -66,6 +66,19 @@ public: malformed_packet() : exception_base("Malformed packet") { } }; +class DNS_decompression_pointer_out_of_bounds : public exception_base { +public: + DNS_decompression_pointer_out_of_bounds() : exception_base("DNS decompression pointer out of bounds") { } +}; + +/** + * \brief Exception thrown when a DNS decompression pointer loops. + */ +class DNS_decompression_pointer_loops : public exception_base { +public: + DNS_decompression_pointer_loops() : exception_base("DNS decompression pointer loops") { } +}; + /** * \brief Exception thrown when serializing a packet fails. */ diff --git a/src/dns.cpp b/src/dns.cpp index 46712e7..759ddde 100644 --- a/src/dns.cpp +++ b/src/dns.cpp @@ -336,7 +336,11 @@ uint32_t DNS::compose_name(const uint8_t* ptr, char* out_ptr) const { const uint8_t* end = &records_data_[0] + records_data_.size(); const uint8_t* end_ptr = 0; char* current_out_ptr = out_ptr; + int pointer_counter = 0; while (*ptr) { + if (pointer_counter++ > 30){ + throw DNS_decompression_pointer_loops(); + } // It's an offset if ((*ptr & 0xc0)) { if (TINS_UNLIKELY(ptr + sizeof(uint16_t) > end)) { @@ -347,7 +351,7 @@ uint32_t DNS::compose_name(const uint8_t* ptr, char* out_ptr) const { index = Endian::be_to_host(index) & 0x3fff; // Check that the offset is neither too low or too high if (index < 0x0c || (&records_data_[0] + (index - 0x0c)) >= end) { - throw malformed_packet(); + throw DNS_decompression_pointer_out_of_bounds(); } // We've probably found the end of the original domain name. Save it. if (end_ptr == 0) { From 1650b60234ad0a2da5fd0ee2e3d948ece571db56 Mon Sep 17 00:00:00 2001 From: Gaya Cohen Date: Mon, 24 May 2021 17:04:11 +0300 Subject: [PATCH 2/4] change counter variable type and add exception description comment --- include/tins/exceptions.h | 3 +++ src/dns.cpp | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/tins/exceptions.h b/include/tins/exceptions.h index 3ef91ba..a2583ae 100644 --- a/include/tins/exceptions.h +++ b/include/tins/exceptions.h @@ -66,6 +66,9 @@ public: malformed_packet() : exception_base("Malformed packet") { } }; +/** + * \brief Exception thrown when a DNS decompression pointer is out of bounds. + */ class DNS_decompression_pointer_out_of_bounds : public exception_base { public: DNS_decompression_pointer_out_of_bounds() : exception_base("DNS decompression pointer out of bounds") { } diff --git a/src/dns.cpp b/src/dns.cpp index 759ddde..a8853eb 100644 --- a/src/dns.cpp +++ b/src/dns.cpp @@ -336,7 +336,7 @@ uint32_t DNS::compose_name(const uint8_t* ptr, char* out_ptr) const { const uint8_t* end = &records_data_[0] + records_data_.size(); const uint8_t* end_ptr = 0; char* current_out_ptr = out_ptr; - int pointer_counter = 0; + uint8_t pointer_counter = 0; while (*ptr) { if (pointer_counter++ > 30){ throw DNS_decompression_pointer_loops(); From ed2b3c12d5c95b4ea8c12c2bac2efd7f32af0d03 Mon Sep 17 00:00:00 2001 From: Gaya Cohen Date: Wed, 9 Jun 2021 11:39:43 +0300 Subject: [PATCH 3/4] Make new exceptions inherit from malformed_packet and change exception names --- include/tins/exceptions.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/tins/exceptions.h b/include/tins/exceptions.h index a2583ae..88fbff8 100644 --- a/include/tins/exceptions.h +++ b/include/tins/exceptions.h @@ -69,17 +69,17 @@ public: /** * \brief Exception thrown when a DNS decompression pointer is out of bounds. */ -class DNS_decompression_pointer_out_of_bounds : public exception_base { +class dns_decompression_pointer_out_of_bounds : public malformed_packet { public: - DNS_decompression_pointer_out_of_bounds() : exception_base("DNS decompression pointer out of bounds") { } + dns_decompression_pointer_out_of_bounds() : exception_base("DNS decompression: pointer out of bounds") { } }; /** * \brief Exception thrown when a DNS decompression pointer loops. */ -class DNS_decompression_pointer_loops : public exception_base { +class dns_decompression_pointer_loops : public malformed_packet { public: - DNS_decompression_pointer_loops() : exception_base("DNS decompression pointer loops") { } + dns_decompression_pointer_loops() : exception_base("DNS decompression: pointer loops") { } }; /** From 137b56d5a7a2e54e160a15b60f2b2298df7279d8 Mon Sep 17 00:00:00 2001 From: Gaya Cohen Date: Wed, 9 Jun 2021 15:57:04 +0300 Subject: [PATCH 4/4] fix exception inheritance and change exception names in DNS code --- include/tins/exceptions.h | 6 ++++-- src/dns.cpp | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/tins/exceptions.h b/include/tins/exceptions.h index 88fbff8..3446718 100644 --- a/include/tins/exceptions.h +++ b/include/tins/exceptions.h @@ -64,6 +64,7 @@ public: class malformed_packet : public exception_base { public: malformed_packet() : exception_base("Malformed packet") { } + malformed_packet(const std::string& message) : exception_base(message) { } }; /** @@ -71,7 +72,7 @@ public: */ class dns_decompression_pointer_out_of_bounds : public malformed_packet { public: - dns_decompression_pointer_out_of_bounds() : exception_base("DNS decompression: pointer out of bounds") { } + dns_decompression_pointer_out_of_bounds() : malformed_packet("DNS decompression: pointer out of bounds") { } }; /** @@ -79,9 +80,10 @@ public: */ class dns_decompression_pointer_loops : public malformed_packet { public: - dns_decompression_pointer_loops() : exception_base("DNS decompression: pointer loops") { } + dns_decompression_pointer_loops() : malformed_packet("DNS decompression: pointer loops") { } }; + /** * \brief Exception thrown when serializing a packet fails. */ diff --git a/src/dns.cpp b/src/dns.cpp index a8853eb..37f03cc 100644 --- a/src/dns.cpp +++ b/src/dns.cpp @@ -339,7 +339,7 @@ uint32_t DNS::compose_name(const uint8_t* ptr, char* out_ptr) const { uint8_t pointer_counter = 0; while (*ptr) { if (pointer_counter++ > 30){ - throw DNS_decompression_pointer_loops(); + throw dns_decompression_pointer_loops(); } // It's an offset if ((*ptr & 0xc0)) { @@ -351,7 +351,7 @@ uint32_t DNS::compose_name(const uint8_t* ptr, char* out_ptr) const { index = Endian::be_to_host(index) & 0x3fff; // Check that the offset is neither too low or too high if (index < 0x0c || (&records_data_[0] + (index - 0x0c)) >= end) { - throw DNS_decompression_pointer_out_of_bounds(); + throw dns_decompression_pointer_out_of_bounds(); } // We've probably found the end of the original domain name. Save it. if (end_ptr == 0) {