lib: make IsAclEntryValid() more robust

Fixes #1175: bareos crashes with invalid character in ACL

Previously IsAclEntryValid() took a char* as a parameter that was
expected to be a POOLMEM (and would have been resized when an error
occured).
This char* has been replaced by a std::vector<char>& and the new
overload for Mmsg() is now used to format a message.

(cherry picked from commit d3983ba)

core/src/dird/dird_conf.cc

@@ -3026,7 +3026,7 @@ static void StoreAcl(LEX* lc, ResourceItem* item, int index, int pass)
   for (;;) {
     LexGetToken(lc, BCT_STRING);
     if (pass == 1) {
-      if (!IsAclEntryValid(lc->str, msg.data())) {
+      if (!IsAclEntryValid(lc->str, msg)) {
         Emsg1(M_ERROR, 0, _("Cannot store Acl: %s\n"), msg.data());
         return;
       }

core/src/lib/edit.cc

@@ -594,15 +594,15 @@ char* add_commas(char* val, char* buf)
  * check if acl entry is valid
  * valid acl entries contain only A-Z 0-9 and !*.:_-'/
  */
-bool IsAclEntryValid(const char* acl, char* msg)
+bool IsAclEntryValid(const char* acl, std::vector<char>& msg)
 {
   int len;
   const char* p;
   const char* accept = "!*.:_-'/"; /* Special characters to accept */
 
 
   if (!acl) {
-    if (msg) { Mmsg(msg, _("Empty acl not allowed.\n")); }
+    Mmsg(msg, _("Empty acl not allowed.\n"));
     return false;
   }
 
@@ -611,18 +611,18 @@ bool IsAclEntryValid(const char* acl, char* msg)
     if (B_ISALPHA(*p) || B_ISDIGIT(*p) || strchr(accept, (int)(*p))) {
       continue;
     }
-    if (msg) { Mmsg(msg, _("Illegal character \"%c\" in acl.\n"), *p); }
+    Mmsg(msg, _("Illegal character \"%c\" in acl.\n"), *p);
     return false;
   }
 
   len = p - acl;
   if (len >= MAX_NAME_LENGTH) {
-    if (msg) { Mmsg(msg, _("Acl too long.\n")); }
+    Mmsg(msg, _("Acl too long.\n"));
     return false;
   }
 
   if (len == 0) {
-    if (msg) { Mmsg(msg, _("Acl must be at least one character long.\n")); }
+    Mmsg(msg, _("Acl must be at least one character long.\n"));
     return false;
   }
 
@@ -631,13 +631,7 @@ bool IsAclEntryValid(const char* acl, char* msg)
 
 bool IsAclEntryValid(const char* acl)
 {
-  bool retval;
-  POOLMEM* msg = GetPoolMemory(PM_NAME);
-
-  retval = IsAclEntryValid(acl, msg);
-
-  FreePoolMemory(msg);
-
-  return retval;
+  std::vector<char> msg;
+  return IsAclEntryValid(acl, msg);
 }
 

core/src/lib/edit.h

@@ -21,6 +21,8 @@
 #ifndef BAREOS_LIB_EDIT_H_
 #define BAREOS_LIB_EDIT_H_
 
+#include <vector>
+
 uint64_t str_to_uint64(const char* str);
 #define str_to_uint16(str) ((uint16_t)str_to_uint64(str))
 #define str_to_uint32(str) ((uint32_t)str_to_uint64(str))
@@ -43,7 +45,7 @@ bool Is_a_number_list(const char* n);
 bool IsAnInteger(const char* n);
 bool IsNameValid(const char* name, POOLMEM*& msg);
 bool IsNameValid(const char* name);
-bool IsAclEntryValid(const char* acl, char* msg);
+bool IsAclEntryValid(const char* acl, std::vector<char>& msg);
 bool IsAclEntryValid(const char* acl);
 
 #endif  // BAREOS_LIB_EDIT_H_

core/src/tests/test_acl_entry_syntax.cc

@@ -28,31 +28,32 @@
 
 #include "lib/edit.h"
 
-POOLMEM* msg = GetPoolMemory(PM_FNAME);
 
 TEST(acl_entry_syntax_test, acl_entry_syntax_test)
 {
+  std::vector<char> msg;
+
   EXPECT_EQ(true, IsAclEntryValid("list", msg));
 
   EXPECT_EQ(false, IsAclEntryValid("list,add", msg));
-  EXPECT_STREQ("Illegal character \",\" in acl.\n", msg);
+  EXPECT_STREQ("Illegal character \",\" in acl.\n", msg.data());
 
   EXPECT_EQ(true, IsAclEntryValid("STRING.CONTAINING.ALLOWED.CHARS!*.", msg));
 
   std::string string_maximum_length(MAX_NAME_LENGTH - 1, '.');
   EXPECT_EQ(true, IsAclEntryValid(string_maximum_length.c_str(), msg));
 
   EXPECT_EQ(false, IsAclEntryValid("illegalch@racter", msg));
-  EXPECT_STREQ("Illegal character \"@\" in acl.\n", msg);
+  EXPECT_STREQ("Illegal character \"@\" in acl.\n", msg.data());
 
   EXPECT_EQ(false, IsAclEntryValid("", msg));
-  EXPECT_STREQ("Acl must be at least one character long.\n", msg);
+  EXPECT_STREQ("Acl must be at least one character long.\n", msg.data());
 
   EXPECT_EQ(false, IsAclEntryValid(nullptr, msg));
-  EXPECT_STREQ("Empty acl not allowed.\n", msg);
+  EXPECT_STREQ("Empty acl not allowed.\n", msg.data());
 
   std::string string_too_long(MAX_NAME_LENGTH, '.');
   EXPECT_EQ(false, IsAclEntryValid(string_too_long.c_str(), msg));
 
-  EXPECT_STREQ("Acl too long.\n", msg);
+  EXPECT_STREQ("Acl too long.\n", msg.data());
 }