dnsdist: Fix invalid `substr()` use in the DNS overlay parser

`std::basic_string_view<CharT,Traits>::substr`'s second parameter is a length,
not an iterator or a position, so the existing code was misusing it and
creating a view that potentially expanded outside of the packet.
However currently the view is never used to read more than
`record.d_contentOffset` (we are passing it immediately to `makeComboAddressFromRaw`
with `record.d_contentLength` as the length) and `record.d_contentOffset`
has been validated right before to be either `4` or `16`, so
there is no out-of-bounds read.
This issue has been introduced in b6f9a21db93ee25ec665dc5f65e87eb7adebd102 and
is not included in any stable release, so no need to backport
the fix.

Reported by Nyaz360 in YWH-PGM6095-85, thanks a lot!

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/dnsdistdist/dnsdist-dnsparser.cc b/pdns/dnsdistdist/dnsdist-dnsparser.cc
index cfcd5ce86d8dd44aa9e6693d0501ea6602a44952..7460d801b6478e1fd619995a1f9fb32e792b76fb 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-dnsparser.cc
+++ b/pdns/dnsdistdist/dnsdist-dnsparser.cc
@@ -248,7 +248,7 @@ namespace RecordParsers
     }
 
     // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage): length is passed in and used to read data
-    return makeComboAddressFromRaw(4, packet.substr(record.d_contentOffset, record.d_contentOffset + 4).data(), record.d_contentLength);
+    return makeComboAddressFromRaw(4, packet.substr(record.d_contentOffset, record.d_contentLength).data(), record.d_contentLength);
   }
 
   std::optional<ComboAddress> parseAAAARecord(const std::string_view& packet, const DNSPacketOverlay::Record& record)
@@ -258,19 +258,19 @@ namespace RecordParsers
     }
 
     // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage): length is passed in and used to read data
-    return makeComboAddressFromRaw(6, packet.substr(record.d_contentOffset, record.d_contentOffset + 16).data(), record.d_contentLength);
+    return makeComboAddressFromRaw(6, packet.substr(record.d_contentOffset, record.d_contentLength).data(), record.d_contentLength);
   }
 
   std::optional<ComboAddress> parseAddressRecord(const std::string_view& packet, const DNSPacketOverlay::Record& record)
   {
     if (record.d_type == QType::A && record.d_contentLength == 4) {
       // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage): length is passed in and used to read data
-      return makeComboAddressFromRaw(4, packet.substr(record.d_contentOffset, record.d_contentOffset + 4).data(), record.d_contentLength);
+      return makeComboAddressFromRaw(4, packet.substr(record.d_contentOffset, record.d_contentLength).data(), record.d_contentLength);
     }
 
     if (record.d_type == QType::AAAA && record.d_contentLength == 16) {
       // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage): length is passed in and used to read data
-      return makeComboAddressFromRaw(6, packet.substr(record.d_contentOffset, record.d_contentOffset + 16).data(), record.d_contentLength);
+      return makeComboAddressFromRaw(6, packet.substr(record.d_contentOffset, record.d_contentLength).data(), record.d_contentLength);
     }
 
     return {};