File v2-6-6-fix-rce.patch of Package cobbler.22866

Overview Repositories Revisions Requests Users Attributes Meta

File v2-6-6-fix-rce.patch of Package cobbler.22866

From 02cdda598214d9e770fce1fcc8174e2fd15efd64 Mon Sep 17 00:00:00 2001
From: SchoolGuy <egotthold@suse.de>
Date: Tue, 24 Aug 2021 11:32:37 +0200
Subject: [PATCH] Backport of the log poisoning and arbitrary read & write
 patch

This should prevent injecting Cheetah or Jinja template language into a logmessage and reading or writing arbitrary files thorugh relative paths.
---
 cobbler/item.py   | 10 ++-----
 cobbler/pxegen.py |  9 +++++-
 cobbler/remote.py | 76 ++++++++++++++++++++++++++++++++++-------------
 cobbler/utils.py  | 34 +++++++++++++++++++++
 4 files changed, 101 insertions(+), 28 deletions(-)

diff --git a/cobbler/item.py b/cobbler/item.py
index 39693654..e056d595 100644
--- a/cobbler/item.py
+++ b/cobbler/item.py
@@ -19,14 +19,12 @@ from cexceptions import *
 from utils import _
 import pprint
 import fnmatch
-import re
+
 
 class Item:
 
     TYPE_NAME = "generic"
 
-    _re_name = re.compile(r'[a-zA-Z0-9_\-.:+]*$')
-
     def __init__(self,config,is_subobject=False):
         """
         Constructor.  Requires a back reference to the Config management object.
@@ -144,10 +142,8 @@ class Item:
 
     def validate_name(self, name):
         """Validate name. Raises CX if the name if invalid"""
-        if not isinstance(name, basestring):
-            raise CX(_("name must be a string"))
-        if not self._re_name.match(name):
-            raise CX(_("invalid characters in name: '%s'" % name))
+        if not utils.validate_obj_name(name):
+            raise CX(_("invalid characters in name or wrong type: '%s'" % name))
 
     def set_name(self,name):
         """
diff --git a/cobbler/pxegen.py b/cobbler/pxegen.py
index b241305d..81c8e6f9 100644
--- a/cobbler/pxegen.py
+++ b/cobbler/pxegen.py
@@ -1230,8 +1230,13 @@ class PXEGen:
     def generate_script(self,what,objname,script_name):
        if what == "profile":
            obj = self.api.find_profile(name=objname)
-       else:
+       elif what == "system":
            obj = self.api.find_system(name=objname)
+       else:
+           raise ValueError("\"what\" needs to be either \"profile\" or \"system\"!")
+
+       if not utils.validate_autoinstall_script_name(script_name):
+           raise ValueError("\"script_name\" handed to generate_script was not valid!")
 
        if not obj:
            return "# %s named %s not found" % (what,objname)
@@ -1259,6 +1264,8 @@ class PXEGen:
 
        scripts_path = "/var/lib/cobbler/scripts"
        template = os.path.normpath(os.path.join(scripts_path,script_name))
+       if not template.startswith(scripts_path):
+           return "# script template \"%s\" could not be found in the script root" % script_name
 
        available_scripts = []
        for root, folders, files in os.walk(scripts_path):
diff --git a/cobbler/remote.py b/cobbler/remote.py
index b5f46981..d255d33b 100644
--- a/cobbler/remote.py
+++ b/cobbler/remote.py
@@ -19,10 +19,11 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
 02110-1301  USA
 """
-
+import keyword
 import sys, socket, time, os, errno, re, random, stat, string
 import base64
 import SimpleXMLRPCServer
+import tokenize
 from SocketServer import ThreadingMixIn
 import xmlrpclib
 import base64
@@ -46,6 +47,7 @@ import clogger
 import pxegen
 import utils
 #from utils import * # BAD!
+from cobbler import item
 from utils import _
 import configgen
 
@@ -404,7 +406,7 @@ class CobblerXMLRPCInterface:
         else:
             return self.token_cache[token][1]
 
-    def _log(self,msg,user=None,token=None,name=None,object_id=None,attribute=None,debug=False,error=False):
+    def _log(self,msg,token=None,name=None,object_id=None,attribute=None,debug=False,error=False):
         """
         Helper function to write data to the log file from the XMLRPC remote implementation.
         Takes various optional parameters that should be supplied when known.
@@ -412,8 +414,6 @@ class CobblerXMLRPCInterface:
 
         # add the user editing the object, if supplied
         m_user = "?"
-        if user is not None:
-           m_user = user
         if token is not None:
            try:
                m_user = self.get_user_from_token(token)
@@ -423,14 +423,22 @@ class CobblerXMLRPCInterface:
         msg = "REMOTE %s; user(%s)" % (msg, m_user)
  
         if name is not None:
+            if not isinstance(name, str):
+                return
+            if not utils.validate_obj_name(name):
+                return
             msg = "%s; name(%s)" % (msg, name)
 
         if object_id is not None:
+            if not utils.validate_obj_id(object_id):
+                return
             msg = "%s; object_id(%s)" % (msg, object_id)
 
         # add any attributes being modified, if any
         if attribute:
-           msg = "%s; attribute(%s)" % (msg, attribute)
+            if not (isinstance(attribute, str) and re.match(tokenize.Name + '$', attribute)) or keyword.iskeyword(attribute):
+                return
+            msg = "%s; attribute(%s)" % (msg, attribute)
         
         # log to the correct logger
         if error:
@@ -863,6 +871,9 @@ class CobblerXMLRPCInterface:
     def modify_file(self,object_id,attribute,arg,token):
         return self.modify_item("file",object_id,attribute,arg,token)
     def modify_setting(self,setting_name,value,token):
+        if not self.api.settings().allow_dynamic_settings:
+            self._log("modify_setting - feature turned off but was tried to be accessed", token=token)
+            return 1
         self._log("modify_setting(%s)" % setting_name, token=token)
         self.check_access(token, "modify_setting")
         try:
@@ -1072,7 +1083,10 @@ class CobblerXMLRPCInterface:
         return self.api.generate_bootcfg(profile,system)
 
     def generate_script(self,profile=None,system=None,name=None,**rest):
-        self._log("generate_script, name is %s" % str(name))
+        # This is duplicated from tftpgen.py to prevent log poisoning via a template engine (Cheetah, Jinja2).
+        if not utils.validate_autoinstall_script_name(name):
+            raise ValueError("\"name\" handed to generate_script was not valid!")
+        self._log("generate_script, name is \"%s\"" % name)
         return self.api.generate_script(profile,system,name)
 
     def get_blended_data(self,profile=None,system=None):
@@ -1287,6 +1301,8 @@ class CobblerXMLRPCInterface:
         As it's a bit of a potential log-flooder, it's off by default
         and needs to be enabled in /etc/cobbler/settings.
         """
+        if not self.__validate_log_data_params(sys_name, file, size, offset, data, token):
+            return False
 
         self._log("upload_log_data (file: '%s', size: %s, offset: %s)" % (file, size, offset), token=token, name=sys_name)
 
@@ -1301,9 +1317,29 @@ class CobblerXMLRPCInterface:
         if obj == None:
             # system not found!
             self._log("upload_log_data - WARNING - system '%s' not found in cobbler" % sys_name, token=token, name=sys_name)
+            return False
 
         return self.__upload_file(sys_name, file, size, offset, data)
 
+    def __validate_log_data_params(self, sys_name, logfile_name, size, offset, data, token=None):
+        # Validate all types
+        if not (isinstance(sys_name, str) and isinstance(logfile_name, str) and isinstance(size, int)
+                and isinstance(offset, int) and isinstance(data, bytes)):
+            self.logger.warning("upload_log_data - One of the parameters handed over had an invalid type!")
+            return False
+        if token is not None and not isinstance(token, str):
+            self.logger.warning("upload_log_data - token was given but had an invalid type.")
+            return False
+        # Validate sys_name with item regex
+        if not re.match(item.Item._re_name, sys_name):
+            self.logger.warning("upload_log_data - The provided sys_name contained invalid characters!")
+            return False
+        # Validate logfile_name - this uses the script name validation, possibly we need our own for this one later
+        if not utils.validate_autoinstall_script_name(logfile_name):
+            self.logger.warning("upload_log_data - The provided file contained invalid characters!")
+            return False
+        return True
+
     def __upload_file(self, sys_name, file, size, offset, data):
         '''
         system: the name of the system
@@ -1322,21 +1358,21 @@ class CobblerXMLRPCInterface:
                 if size != len(contents): 
                     return False
 
-        #XXX - have an incoming dir and move after upload complete
-        # SECURITY - ensure path remains under uploadpath
-        tt = string.maketrans("/","+")
-        fn = string.translate(file, tt)
-        if fn.startswith('..'):
-            raise CX("invalid filename used: %s" % fn)
+        anamon_base_directory = "/var/log/cobbler/anamon"
+        anamon_sys_directory = os.path.join(anamon_base_directory, sys_name)
+
+        file_name = os.path.join(anamon_sys_directory, file)
+        normalized_path = os.path.normpath(file_name)
+        if not normalized_path.startswith(anamon_sys_directory):
+            self.logger.warning("upload_log_data: built path for the logfile was outside of the Cobbler-Anamon log "
+                                "directory!")
+            return False
 
-        # FIXME ... get the base dir from cobbler settings()
-        udir = "/var/log/cobbler/anamon/%s" % sys_name
-        if not os.path.isdir(udir):
-            os.mkdir(udir, 0755)
+        if not os.path.isdir(anamon_sys_directory):
+            os.mkdir(anamon_sys_directory, 0o755)
 
-        fn = "%s/%s" % (udir, fn)
         try:
-            st = os.lstat(fn)
+            st = os.lstat(file_name)
         except OSError, e:
             if e.errno == errno.ENOENT:
                 pass
@@ -1344,9 +1380,9 @@ class CobblerXMLRPCInterface:
                 raise
         else:
             if not stat.S_ISREG(st.st_mode):
-                raise CX("destination not a file: %s" % fn)
+                raise CX("destination not a file: %s" % file_name)
 
-        fd = os.open(fn, os.O_RDWR | os.O_CREAT, 0644)
+        fd = os.open(file_name, os.O_RDWR | os.O_CREAT, 0644)
         # log_error("fd=%r" %fd)
         try:
             if offset == 0 or (offset == -1 and size == len(contents)):
diff --git a/cobbler/utils.py b/cobbler/utils.py
index cb431964..615616ba 100644
--- a/cobbler/utils.py
+++ b/cobbler/utils.py
@@ -110,6 +110,7 @@ _re_kernel = re.compile(r'(vmlinu[xz]|kernel.img)')
 _re_initrd = re.compile(r'(initrd(.*).img|ramdisk.image.gz)')
 _re_is_mac = re.compile(':'.join(('[0-9A-Fa-f][0-9A-Fa-f]',)*6) + '$')
 _re_is_ibmac = re.compile(':'.join(('[0-9A-Fa-f][0-9A-Fa-f]',)*20) + '$')
+_re_name = re.compile(r'[a-zA-Z0-9_\-.:+]*$')
 
 # all logging from utils.die goes to the main log even if there
 # is another log.
@@ -2331,6 +2332,39 @@ def suse_kopts_textmode_overwrite(distro, kopts):
             kopts.pop('text', None)
             kopts['textmode'] = ['1']
 
+
+RE_SCRIPT_NAME = re.compile(r"(?:[a-zA-Z0-9_.]+)\Z")
+
+
+def validate_autoinstall_script_name(name):
+    if not isinstance(name, str):
+        return False
+    if re.match(RE_SCRIPT_NAME, name):
+        return True
+    return False
+
+
+def validate_obj_name(object_name):
+    if not isinstance(object_name, basestring):
+        return False
+    return bool(re.match(_re_name, object_name))
+
+
+def validate_obj_type(object_type):
+    if not isinstance(object_type, str):
+        return False
+    return object_type in ["distro", "profile", "system", "repo", "image", "mgmtclass", "package", "file"]
+
+
+def validate_obj_id(object_id):
+    if not isinstance(object_id, str):
+        return False
+    if object_id.startswith("___NEW___"):
+        object_id = object_id[9:]
+    (otype, oname) = object_id.split("::", 1)
+    return validate_obj_type(otype) and validate_obj_name(oname)
+
+
 if __name__ == "__main__":
     print os_release() # returns 2, not 3
 
-- 
2.32.0

Places

File v2-6-6-fix-rce.patch of Package cobbler.22866

Places