Merge branch 'pu/fuzzerpatches' into 'master'

Various patches uplifted from unfinished fuzzer branches

See merge request apt-team/apt!158

5 files changed, 119 insertions, 84 deletions

diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index 7038131cf..bd50e8078 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -325,7 +325,11 @@ protected:
 
       rc = seccomp_load(ctx);
       if (rc == -EINVAL)
-	 Warning("aptMethod::Configuration: could not load seccomp policy: %s", strerror(-rc));
+      {
+	 std::string msg;
+	 strprintf(msg, "aptMethod::Configuration: could not load seccomp policy: %s", strerror(-rc));
+	 Warning(std::move(msg));
+      }
       else if (rc != 0)
 	 return _error->FatalE("aptMethod::Configuration", "could not load seccomp policy: %s", strerror(-rc));
 
@@ -380,12 +384,17 @@ protected:
       return true;
    }
 
-   void Warning(const char *Format,...)
+   void Warning(std::string &&msg)
    {
-      va_list args;
-      va_start(args,Format);
-      PrintStatus("104 Warning", Format, args);
-      va_end(args);
+      std::unordered_map<std::string, std::string> fields;
+      if (Queue != 0)
+	 fields.emplace("URI", Queue->Uri);
+      else
+	 fields.emplace("URI", "<UNKNOWN>");
+      if (not UsedMirror.empty())
+	 fields.emplace("UsedMirror", UsedMirror);
+      fields.emplace("Message", std::move(msg));
+      SendMessage("104 Warning", std::move(fields));
    }
 
    std::vector<std::string> methodNames;
@@ -560,14 +569,11 @@ class aptAuthConfMethod : public aptMethod
 	 result &= MaybeAddAuth(*authconf, uri);
       }
 
-      if (not _error->empty())
+      while (not _error->empty())
       {
 	 std::string message;
-	 while (not _error->empty())
-	 {
-	    _error->PopMessage(message);
-	    Warning("%s", message.c_str());
-	 }
+	 _error->PopMessage(message);
+	 Warning(std::move(message));
       }
       _error->RevertToStack();
 
diff --git a/methods/basehttp.cc b/methods/basehttp.cc
index 8aac1090c..3786e2e6c 100644
--- a/methods/basehttp.cc
+++ b/methods/basehttp.cc
@@ -110,6 +110,9 @@ bool RequestState::HeaderLine(string const &Line)			/*{{{*/
 	 if (sscanf(Line.c_str(),"HTTP %3u%359[^\n]",&Result,Code) != 2)
 	    return _error->Error(_("The HTTP server sent an invalid reply header"));
       }
+      auto const CodeLen = strlen(Code);
+      auto const CodeEnd = std::remove_if(Code, Code + CodeLen, [](char c) { return isprint(c) == 0; });
+      *CodeEnd = '\0';
 
       /* Check the HTTP response header to get the default persistence
          state. */
@@ -762,7 +765,9 @@ int BaseHttpMethod::Loop()
 			// yes, he did! Disable pipelining and rewrite queue
 			if (Server->Pipeline == true)
 			{
-			   Warning(_("Automatically disabled %s due to incorrect response from server/proxy. (man 5 apt.conf)"), "Acquire::http::Pipeline-Depth");
+			   std::string msg;
+			   strprintf(msg, _("Automatically disabled %s due to incorrect response from server/proxy. (man 5 apt.conf)"), "Acquire::http::Pipeline-Depth");
+			   Warning(std::move(msg));
 			   Server->Pipeline = false;
 			   Server->PipelineAllowed = false;
 			   // we keep the PipelineDepth value so that the rest of the queue can be fixed up as well
diff --git a/methods/basehttp.h b/methods/basehttp.h
index 62c9963ea..c0d14d854 100644
--- a/methods/basehttp.h
+++ b/methods/basehttp.h
@@ -60,7 +60,7 @@ struct RequestState
    bool AddPartialFileToHashes(FileFd &File);
 
    RequestState(BaseHttpMethod * const Owner, ServerState * const Server) :
-      Owner(Owner), Server(Server) { time(&Date); }
+      Owner(Owner), Server(Server) { time(&Date); Code[0] = '\0'; }
 };
 struct ServerState
 {
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 08d030a17..a9da456ec 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -264,7 +264,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
 	    SubKeyMapping[tokens[9]].emplace_back(sig);
       }
       else if (strncmp(buffer, APTKEYWARNING, sizeof(APTKEYWARNING)-1) == 0)
-         Warning("%s", buffer + sizeof(APTKEYWARNING));
+         Warning(buffer + sizeof(APTKEYWARNING));
       else if (strncmp(buffer, APTKEYERROR, sizeof(APTKEYERROR)-1) == 0)
 	 _error->Error("%s", buffer + sizeof(APTKEYERROR));
    }
@@ -442,8 +442,12 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
 	    }))
    {
       for (auto const & Signer : Signers.SoonWorthless)
+      {
+	 std::string msg;
          // TRANSLATORS: The second %s is the reason and is untranslated for repository owners.
-         Warning(_("Signature by key %s uses weak digest algorithm (%s)"), Signer.key.c_str(), Signer.note.c_str());
+	 strprintf(msg, _("Signature by key %s uses weak digest algorithm (%s)"), Signer.key.c_str(), Signer.note.c_str());
+         Warning(std::move(msg));
+      }
    }
 
    if (Signers.Good.empty() || !Signers.Bad.empty() || !Signers.NoPubKey.empty())
diff --git a/methods/rred.cc b/methods/rred.cc
index 981364a9e..f53f05ad5 100644
--- a/methods/rred.cc
+++ b/methods/rred.cc
@@ -7,12 +7,15 @@
 
 #include <config.h>
 
+#ifndef APT_EXCLUDE_RRED_METHOD_CODE
 #include "aptmethod.h"
 #include <apt-pkg/configuration.h>
+#include <apt-pkg/init.h>
+#endif
+
 #include <apt-pkg/error.h>
 #include <apt-pkg/fileutl.h>
 #include <apt-pkg/hashes.h>
-#include <apt-pkg/init.h>
 #include <apt-pkg/strutl.h>
 
 #include <apt-private/private-cmndline.h>
@@ -23,7 +26,7 @@
 #include <vector>
 #include <stddef.h>
 
-#include <assert.h>
+#include <cassert>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -33,7 +36,9 @@
 
 #include <apti18n.h>
 
-#define BLOCK_SIZE (512*1024)
+#ifndef APT_MEMBLOCK_SIZE
+#define APT_MEMBLOCK_SIZE (512*1024)
+#endif
 
 static bool ShowHelp(CommandLine &)
 {
@@ -66,9 +71,9 @@ class MemBlock {
    char *start;
    size_t size;
    char *free;
-   MemBlock *next;
+   MemBlock *next = nullptr;
 
-   explicit MemBlock(size_t size) : size(size), next(NULL)
+   explicit MemBlock(size_t size) : size(size)
    {
       free = start = new char[size];
    }
@@ -77,11 +82,7 @@ class MemBlock {
 
    public:
 
-   MemBlock(void) {
-      free = start = new char[BLOCK_SIZE];
-      size = BLOCK_SIZE;
-      next = NULL;
-   }
+   MemBlock() : MemBlock(APT_MEMBLOCK_SIZE) {}
 
    ~MemBlock() {
       delete [] start;
@@ -100,11 +101,9 @@ class MemBlock {
 	 for (MemBlock *k = this; k; k = k->next) {
 	    if (k->free == last) {
 	       if (len <= k->avail()) {
-		  char *n = k->add(src, len);
-		  assert(last == n);
-		  if (last == n)
-		     return NULL;
-		  return n;
+		  char * const n = k->add(src, len);
+		  assert(last == n); // we checked already that the block is big enough, so a new one shouldn't be used
+		  return (last == n) ? nullptr : n;
 	       } else {
 		  break;
 	       }
@@ -119,10 +118,10 @@ class MemBlock {
    char *add(char *src, size_t len) {
       if (len > avail()) {
 	 if (!next) {
-	    if (len > BLOCK_SIZE)  {
+	    if (len > APT_MEMBLOCK_SIZE)  {
 	       next = new MemBlock(len);
 	    } else {
-	       next = new MemBlock;
+	       next = new MemBlock();
 	    }
 	 }
 	 return next->add(src, len);
@@ -155,24 +154,27 @@ struct Change {
    }
 
    /* actually, don't write <lines> lines from <add> */
-   void skip_lines(size_t lines)
+   bool skip_lines(size_t lines)
    {
       while (lines > 0) {
 	 char *s = (char*) memchr(add, '\n', add_len);
-	 assert(s != NULL);
+	 if (s == nullptr)
+	    return _error->Error("No line left in add_len data to skip (1)");
 	 s++;
 	 add_len -= (s - add);
 	 add_cnt--;
 	 lines--;
 	 if (add_len == 0) {
-	    add = NULL;
-	    assert(add_cnt == 0);
-	    assert(lines == 0);
+	    add = nullptr;
+	    if (add_cnt != 0 || lines != 0)
+	       return _error->Error("No line left in add_len data to skip (2)");
 	 } else {
 	    add = s;
-	    assert(add_cnt > 0);
+	    if (add_cnt == 0)
+	       return _error->Error("No line left in add_len data to skip (3)");
 	 }
       }
+      return true;
    }
 };
 
@@ -183,7 +185,8 @@ class FileChanges {
 
    bool pos_is_okay(void) const
    {
-#ifdef POSDEBUG
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+      // this isn't unsafe, it is just a moderately expensive check we want to avoid normally
       size_t cpos = 0;
       std::list<struct Change>::const_iterator x;
       for (x = changes.begin(); x != where; ++x) {
@@ -208,33 +211,40 @@ class FileChanges {
    std::list<struct Change>::reverse_iterator rbegin(void) { return changes.rbegin(); }
    std::list<struct Change>::reverse_iterator rend(void) { return changes.rend(); }
 
-   void add_change(Change c) {
+   bool add_change(Change c) {
       assert(pos_is_okay());
-      go_to_change_for(c.offset);
-      assert(pos + where->offset == c.offset);
+      if (not go_to_change_for(c.offset) ||
+	  pos + where->offset != c.offset)
+	 return false;
       if (c.del_cnt > 0)
-	 delete_lines(c.del_cnt);
-      assert(pos + where->offset == c.offset);
+	 if (not delete_lines(c.del_cnt))
+	    return false;
+      if (pos + where->offset != c.offset)
+	 return false;
       if (c.add_len > 0) {
 	 assert(pos_is_okay());
 	 if (where->add_len > 0)
-	    new_change();
-	 assert(where->add_len == 0 && where->add_cnt == 0);
+	    if (not new_change())
+	       return false;
+	 if (where->add_len != 0 || where->add_cnt != 0)
+	    return false;
 
 	 where->add_len = c.add_len;
 	 where->add_cnt = c.add_cnt;
 	 where->add = c.add;
       }
       assert(pos_is_okay());
-      merge();
-      assert(pos_is_okay());
+      if (not merge())
+	 return false;
+      return pos_is_okay();
    }
 
    private:
-   void merge(void)
+   bool merge(void)
    {
       while (where->offset == 0 && where != changes.begin()) {
-	 left();
+	 if (not left())
+	    return false;
       }
       std::list<struct Change>::iterator next = where;
       ++next;
@@ -253,53 +263,56 @@ class FileChanges {
 	    ++next;
 	 }
       }
+      return true;
    }
 
-   void go_to_change_for(size_t line)
+   bool go_to_change_for(size_t line)
    {
       while(where != changes.end()) {
 	 if (line < pos) {
-	    left();
+	    if (not left())
+	       return false;
 	    continue;
 	 }
 	 if (pos + where->offset + where->add_cnt <= line) {
-	    right();
+	    if (not right())
+	       return false;
 	    continue;
 	 }
 	 // line is somewhere in this slot
 	 if (line < pos + where->offset) {
 	    break;
 	 } else if (line == pos + where->offset) {
-	    return;
+	    return true;
 	 } else {
-	    split(line - pos);
-	    right();
-	    return;
+	    if (not split(line - pos))
+	       return false;
+	    return right();
 	 }
       }
       /* it goes before this patch */
-      insert(line-pos);
+      return insert(line-pos);
    }
 
-   void new_change(void) { insert(where->offset); }
+   bool new_change(void) { return insert(where->offset); }
 
-   void insert(size_t offset)
+   bool insert(size_t offset)
    {
       assert(pos_is_okay());
-      assert(where == changes.end() || offset <= where->offset);
+      if (where != changes.end() && offset > where->offset)
+	 return false;
       if (where != changes.end())
 	 where->offset -= offset;
       changes.insert(where, Change(offset));
       --where;
-      assert(pos_is_okay());
+      return pos_is_okay();
    }
 
-   void split(size_t offset)
+   bool split(size_t offset)
    {
       assert(pos_is_okay());
-
-      assert(where->offset < offset);
-      assert(offset < where->offset + where->add_cnt);
+      if (where->offset >= offset || offset >= where->offset + where->add_cnt)
+	 return false;
 
       size_t keep_lines = offset - where->offset;
 
@@ -307,27 +320,29 @@ class FileChanges {
 
       where->del_cnt = 0;
       where->offset = 0;
-      where->skip_lines(keep_lines);
+      if (not where->skip_lines(keep_lines))
+	 return false;
 
       before.add_cnt = keep_lines;
       before.add_len -= where->add_len;
 
       changes.insert(where, before);
       --where;
-      assert(pos_is_okay());
+      return pos_is_okay();
    }
 
-   void delete_lines(size_t cnt)
+   bool delete_lines(size_t cnt)
    {
-      std::list<struct Change>::iterator x = where;
       assert(pos_is_okay());
+      std::list<struct Change>::iterator x = where;
       while (cnt > 0)
       {
 	 size_t del;
 	 del = x->add_cnt;
 	 if (del > cnt)
 	    del = cnt;
-	 x->skip_lines(del);
+	 if (not x->skip_lines(del))
+	    return false;
 	 cnt -= del;
 
 	 ++x;
@@ -342,21 +357,21 @@ class FileChanges {
 	 where->del_cnt += del;
 	 cnt -= del;
       }
-      assert(pos_is_okay());
+      return pos_is_okay();
    }
 
-   void left(void) {
+   bool left(void) {
       assert(pos_is_okay());
       --where;
       pos -= where->offset + where->add_cnt;
-      assert(pos_is_okay());
+      return pos_is_okay();
    }
 
-   void right(void) {
+   bool right(void) {
       assert(pos_is_okay());
       pos += where->offset + where->add_cnt;
       ++where;
-      assert(pos_is_okay());
+      return pos_is_okay();
    }
 };
 
@@ -378,7 +393,7 @@ class Patch {
    static void dump_rest(FileFd &o, FileFd &i,
 	 Hashes * const start_hash, Hashes * const end_hash)
    {
-      char buffer[BLOCK_SIZE];
+      char buffer[APT_MEMBLOCK_SIZE];
       unsigned long long l = 0;
       while (i.Read(buffer, sizeof(buffer), &l)) {
 	 if (l ==0  || !retry_fwrite(buffer, l, o, start_hash, end_hash))
@@ -389,7 +404,7 @@ class Patch {
    static void dump_lines(FileFd &o, FileFd &i, size_t n,
 	 Hashes * const start_hash, Hashes * const end_hash)
    {
-      char buffer[BLOCK_SIZE];
+      char buffer[APT_MEMBLOCK_SIZE];
       while (n > 0) {
 	 if (i.ReadLine(buffer, sizeof(buffer)) == NULL)
 	    buffer[0] = '\0';
@@ -402,7 +417,7 @@ class Patch {
 
    static void skip_lines(FileFd &i, int n, Hashes * const start_hash)
    {
-      char buffer[BLOCK_SIZE];
+      char buffer[APT_MEMBLOCK_SIZE];
       while (n > 0) {
 	 if (i.ReadLine(buffer, sizeof(buffer)) == NULL)
 	    buffer[0] = '\0';
@@ -422,7 +437,7 @@ class Patch {
 
    bool read_diff(FileFd &f, Hashes * const h)
    {
-      char buffer[BLOCK_SIZE];
+      char buffer[APT_MEMBLOCK_SIZE];
       bool cmdwanted = true;
 
       Change ch(std::numeric_limits<size_t>::max());
@@ -478,7 +493,8 @@ class Patch {
 		  ch.add = NULL;
 		  ch.add_cnt = 0;
 		  ch.add_len = 0;
-		  filechanges.add_change(ch);
+		  if (not filechanges.add_change(ch))
+		     return _error->Error("Parsing patchfile %s failed: Delete command could not be added to changes", f.Name().c_str());
 		  break;
 	       default:
 		  return _error->Error("Parsing patchfile %s failed: Unknown command", f.Name().c_str());
@@ -486,7 +502,8 @@ class Patch {
 	 } else { /* !cmdwanted */
 	    if (strcmp(buffer, ".\n") == 0) {
 	       cmdwanted = true;
-	       filechanges.add_change(ch);
+	       if (not filechanges.add_change(ch))
+		  return _error->Error("Parsing patchfile %s failed: Data couldn't be added for command (1)", f.Name().c_str());
 	    } else {
 	       char *last = NULL;
 	       char *add;
@@ -500,7 +517,8 @@ class Patch {
 		  ch.add_cnt++;
 	       } else {
 		  if (ch.add) {
-		     filechanges.add_change(ch);
+		     if (not filechanges.add_change(ch))
+			return _error->Error("Parsing patchfile %s failed: Data couldn't be added for command (2)", f.Name().c_str());
 		     ch.del_cnt = 0;
 		  }
 		  ch.offset += ch.add_cnt;
@@ -575,6 +593,7 @@ class Patch {
    }
 };
 
+#ifndef APT_EXCLUDE_RRED_METHOD_CODE
 class RredMethod : public aptMethod {
    private:
       bool Debug;
@@ -859,3 +878,4 @@ int main(int argc, const char *argv[])
    input.Close();
    return DispatchCommandLine(CmdL, {});
 }
+#endif