File v2-6-6-fix-rce.patch of Package cobbler
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
