Avoid overstepping bounds in config file parsing

Our configuration files are not security relevant, but having a parser
which avoids crashing on them even if they are seriously messed up is
not a bad idea anyway. It is also a good opportunity to brush up the
code a bit avoiding a few small string copies with our string_view.

3 files changed, 88 insertions, 60 deletions

diff --git a/apt-pkg/contrib/configuration.cc b/apt-pkg/contrib/configuration.cc
index 554307324..a8fced724 100644
--- a/apt-pkg/contrib/configuration.cc
+++ b/apt-pkg/contrib/configuration.cc
@@ -21,6 +21,7 @@
 #include <apt-pkg/fileutl.h>
 #include <apt-pkg/macros.h>
 #include <apt-pkg/strutl.h>
+#include <apt-pkg/string_view.h>
 
 #include <ctype.h>
 #include <regex.h>
@@ -867,83 +868,76 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio
 
       // Now strip comments; if the whole line is contained in a
       // comment, skip this line.
+      APT::StringView Line{Input.data(), Input.size()};
 
-      // The first meaningful character in the current fragment; will
-      // be adjusted below as we remove bytes from the front.
-      std::string::const_iterator Start = Input.begin();
-      // The last meaningful character in the current fragment.
-      std::string::const_iterator End = Input.end();
-
-      // Multi line comment
-      if (InComment == true)
+      // continued Multi line comment
+      if (InComment)
       {
-	for (std::string::const_iterator I = Start;
-	     I != End; ++I)
+	 size_t end = Line.find("*/");
+	 if (end != APT::StringView::npos)
 	 {
-	    if (*I == '*' && I + 1 != End && I[1] == '/')
-	    {
-	       Start = I + 2;
-	       InComment = false;
-	       break;
-	    }
+	    Line.remove_prefix(end + 2);
+	    InComment = false;
 	 }
-	 if (InComment == true)
+	 else
 	    continue;
       }
 
       // Discard single line comments
-      bool InQuote = false;
-      for (std::string::const_iterator I = Start;
-	   I != End; ++I)
       {
-	 if (*I == '"')
-	    InQuote = !InQuote;
-	 if (InQuote == true)
-	    continue;
-
-	 if ((*I == '/' && I + 1 != End && I[1] == '/') ||
-	     (*I == '#' && strcmp(string(I,I+6).c_str(),"#clear") != 0 &&
-	      strcmp(string(I,I+8).c_str(),"#include") != 0 &&
-	      strcmp(string(I,I+strlen("#x-apt-configure-index")).c_str(), "#x-apt-configure-index") != 0))
+	 size_t start = 0;
+	 while ((start = Line.find("//", start)) != APT::StringView::npos)
+	 {
+	    if (std::count(Line.begin(), Line.begin() + start, '"') % 2 != 0)
+	    {
+	       ++start;
+	       continue;
+	    }
+	    Line.remove_suffix(Line.length() - start);
+	    break;
+	 }
+	 using APT::operator""_sv;
+	 constexpr std::array<APT::StringView, 3> magicComments { "clear"_sv, "include"_sv, "x-apt-configure-index"_sv };
+	 start = 0;
+	 while ((start = Line.find('#', start)) != APT::StringView::npos)
 	 {
-	    End = I;
+	    if (std::count(Line.begin(), Line.begin() + start, '"') % 2 != 0 ||
+		std::any_of(magicComments.begin(), magicComments.end(), [&](auto const m) { return Line.compare(start+1, m.length(), m) == 0; }))
+	    {
+	       ++start;
+	       continue;
+	    }
+	    Line.remove_suffix(Line.length() - start);
 	    break;
 	 }
       }
 
       // Look for multi line comments and build up the
       // fragment.
-      Fragment.reserve(End - Start);
-      InQuote = false;
-      for (std::string::const_iterator I = Start;
-	   I != End; ++I)
+      Fragment.reserve(Line.length());
       {
-	 if (*I == '"')
-	    InQuote = !InQuote;
-	 if (InQuote == true)
-	   Fragment.push_back(*I);
-	 else if (*I == '/' && I + 1 != End && I[1] == '*')
-         {
-	    InComment = true;
-	    for (std::string::const_iterator J = I;
-		 J != End; ++J)
+	 size_t start = 0;
+	 while ((start = Line.find("/*", start)) != APT::StringView::npos)
+	 {
+	    if (std::count(Line.begin(), Line.begin() + start, '"') % 2 != 0)
 	    {
-	       if (*J == '*' && J + 1 != End && J[1] == '/')
-	       {
-		  // Pretend we just finished walking over the
-		  // comment, and don't add anything to the output
-		  // fragment.
-		  I = J + 1;
-		  InComment = false;
-		  break;
-	       }
+	       start += 2;
+	       continue;
 	    }
-
-	    if (InComment == true)
-	      break;
+	    Fragment.append(Line.data(), start);
+	    auto const end = Line.find("*/", start + 2);
+	    if (end == APT::StringView::npos)
+	    {
+	       Line.clear();
+	       InComment = true;
+	       break;
+	    }
+	    else
+	       Line.remove_prefix(end + 2);
+	    start = 0;
 	 }
-	 else
-	   Fragment.push_back(*I);
+	 if (not Line.empty())
+	    Fragment.append(Line.data(), Line.length());
       }
 
       // Skip blank lines.
@@ -951,9 +945,9 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio
 	 continue;
 
       // The line has actual content; interpret what it means.
-      InQuote = false;
-      Start = Fragment.begin();
-      End = Fragment.end();
+      bool InQuote = false;
+      auto Start = Fragment.cbegin();
+      auto End = Fragment.cend();
       for (std::string::const_iterator I = Start;
 	   I != End; ++I)
       {
diff --git a/apt-pkg/contrib/string_view.h b/apt-pkg/contrib/string_view.h
index 05aad3327..04f6ff115 100644
--- a/apt-pkg/contrib/string_view.h
+++ b/apt-pkg/contrib/string_view.h
@@ -39,6 +39,10 @@ public:
     StringView(const char *data) : data_(data), size_(strlen(data)) {}
     StringView(std::string const & str): data_(str.data()), size_(str.size()) {}
 
+    /* Modifiers */
+    void remove_prefix(size_t n) { data_ += n; size_ -= n; }
+    void remove_suffix(size_t n) { size_ -= n; }
+    void clear() { size_ = 0; }
 
     /* Viewers */
     constexpr StringView substr(size_t pos, size_t n = npos) const {
@@ -76,6 +80,28 @@ public:
         return found - data_;
     }
 
+    size_t find(APT::StringView const needle) const {
+       if (needle.empty())
+	  return npos;
+       if (needle.length() == 1)
+	  return find(*needle.data());
+       size_t found = 0;
+       while ((found = find(*needle.data(), found)) != npos) {
+	  if (compare(found, needle.length(), needle) == 0)
+	     return found;
+	  ++found;
+       }
+       return found;
+    }
+    size_t find(APT::StringView const needle, size_t pos) const {
+       if (pos == 0)
+	  return find(needle);
+       size_t const found = substr(pos).find(needle);
+       if (found == npos)
+	  return npos;
+       return pos + found;
+    }
+
     /* Conversions */
     std::string to_string() const {
         return std::string(data_, size_);
diff --git a/test/libapt/stringview_test.cc b/test/libapt/stringview_test.cc
index 03d82517b..9cfaa3b48 100644
--- a/test/libapt/stringview_test.cc
+++ b/test/libapt/stringview_test.cc
@@ -74,6 +74,14 @@ TEST(StringViewTest,Find)
    EXPECT_EQ(defString.to_string().find('e',3), defString.find('e',3));
    EXPECT_EQ(defString.to_string().find('l',6), defString.find('l',6));
    EXPECT_EQ(defString.to_string().find('l',11), defString.find('l',11));
+
+   EXPECT_EQ(defString.to_string().find("l"), defString.find("l"));
+   EXPECT_EQ(defString.to_string().find("ll"), defString.find("ll"));
+   EXPECT_EQ(defString.to_string().find("lo"), defString.find("lo"));
+   EXPECT_EQ(defString.to_string().find("ll", 1), defString.find("ll", 1));
+   EXPECT_EQ(defString.to_string().find("ll", 6), defString.find("ll", 6));
+   EXPECT_EQ(defString.to_string().find("or"), defString.find("or"));
+   EXPECT_EQ(defString.to_string().find("od"), defString.find("od"));
 }
 
 TEST(StringViewTest,RFind)