File bnc_877009.patch of Package cobbler.208

Overview Repositories Revisions Requests Users Attributes Meta

File bnc_877009.patch of Package cobbler.208

From 45d63246ef628ab3f527168aa3e87e753e636cf0 Mon Sep 17 00:00:00 2001
From: Flavio Castelli <fcastelli@suse.com>
Date: Tue, 15 Jul 2014 11:36:30 +0200
Subject: [PATCH] Bug 877009 - VUL-0: CVE-2014-3225: cobbler: Local file
 inclusion

---
 cobbler/remote.py        | 236 +++++++++++++++++++++++---------------
 web/cobbler_web/views.py | 288 ++++++++++++++++++++++++-----------------------
 2 files changed, 295 insertions(+), 229 deletions(-)

diff --git a/cobbler/remote.py b/cobbler/remote.py
index 5ce2350..bafde16 100644
--- a/cobbler/remote.py
+++ b/cobbler/remote.py
@@ -68,6 +68,9 @@ REMAP_COMPAT = {
    "netboot-enabled" : "netboot_enabled"
 }
 
+KICKSTART_TEMPLATE_BASE_DIR = "/var/lib/cobbler/kickstarts/"
+KICKSTART_SNIPPET_BASE_DIR = "/var/lib/cobbler/snippets/"
+
 class CobblerThread(Thread):
     def __init__(self,event_id,remote,logatron,options):
         Thread.__init__(self)
@@ -1889,107 +1892,164 @@ class CobblerXMLRPCInterface:
         self.check_access(token,"sync")
         return self.api.sync()
 
-    def read_or_write_kickstart_template(self,kickstart_file,is_read,new_data,token):
+    def _validate_ks_template_path(self, path):
         """
-        Allows the web app to be used as a kickstart file editor.  For security
-        reasons we will only allow kickstart files to be edited if they reside in
-        /var/lib/cobbler/kickstarts/ or /etc/cobbler.  This limits the damage
-        doable by Evil who has a cobbler password but not a system password.
-        Also if living in /etc/cobbler the file must be a kickstart file.
+        Validate a kickstart template file path
+
+        @param str path kickstart template file path
         """
 
-        if is_read:
-           what = "read_kickstart_template"
-        else:
-           what = "write_kickstart_template"
+        if path.find("..") != -1 or not path.startswith("/"):
+            utils.die(self.logger, "Invalid kickstart template file location %s" % path)
 
-        self._log(what,name=kickstart_file,token=token)
-        self.check_access(token,what,kickstart_file,is_read)
- 
-        if kickstart_file.find("..") != -1 or not kickstart_file.startswith("/"):
-            utils.die(self.logger,"tainted file location")
+        if not path.startswith(KICKSTART_TEMPLATE_BASE_DIR):
+            error = "Invalid kickstart template file location %s, it is not inside %s" % (path, KICKSTART_TEMPLATE_BASE_DIR)
+            utils.die(self.logger, error)
 
-        if not kickstart_file.startswith("/etc/cobbler/") and not kickstart_file.startswith("/var/lib/cobbler/kickstarts"):
-            utils.die(self.logger, "unable to view or edit kickstart in this location")
-        
-        if kickstart_file.startswith("/etc/cobbler/"):
-           if not kickstart_file.endswith(".ks") and not kickstart_file.endswith(".cfg"):
-              # take care to not allow config files to be altered.
-              utils.die(self.logger, "this does not seem to be a kickstart file")
-           if not is_read and not os.path.exists(kickstart_file):
-              utils.die(self.logger, "new files must go in /var/lib/cobbler/kickstarts")
-        
-        if is_read:
-            fileh = open(kickstart_file,"r")
-            data = fileh.read()
-            fileh.close()
-            return data
-        else:
-            if new_data == -1:
-                # delete requested
-                if not self.is_kickstart_in_use(kickstart_file,token):
-                    os.remove(kickstart_file)
-                else:
-                    utils.die(self.logger, "attempt to delete in-use file")
-            else:
-                fileh = open(kickstart_file,"w+")
-                fileh.write(new_data)
-                fileh.close()
-            return True
+    def read_kickstart_template_file(self, file_path, token):
+        """
+        Read a kickstart template file
 
-    def read_or_write_snippet(self,snippet_file,is_read,new_data,token):
+        @param str file_path kickstart template file path
+        @param ? token
+        @return str file content
         """
-        Allows the WebUI to be used as a snippet file editor.  For security
-        reasons we will only allow snippet files to be edited if they reside in
-        /var/lib/cobbler/snippets.
+
+        what = "read_kickstart_template"
+        self._log(what, name=file_path, token=token)
+        self.check_access(token, what, file_path, True)
+        self._validate_ks_template_path(file_path)
+
+        fileh = open(file_path, "r")
+        data = fileh.read()
+        fileh.close()
+
+        return data
+
+    def write_kickstart_template_file(self, file_path, data, token):
         """
-        # FIXME: duplicate code with kickstart view/edit
-        # FIXME: need to move to API level functions
+        Write a kickstart template file
 
-        if is_read:
-           what = "read_snippet"
-        else:
-           what = "write_snippet"
+        @param str file_path kickstart template file path
+        @param str data new file content
+        @param ? token
+        @return bool if operation was successful
+        """
 
-        self._log(what,name=snippet_file,token=token)
-        self.check_access(token,what,snippet_file,is_read)
- 
-        if snippet_file.find("..") != -1 or not snippet_file.startswith("/"):
-            utils.die(self.logger, "tainted file location")
+        what = "write_kickstart_template"
+        self._log(what, name=file_path, token=token)
+        self.check_access(token, what, file_path, True)
+        self._validate_ks_template_path(file_path)
 
-        # FIXME: shouldn't we get snippetdir from the settings?
-        if not snippet_file.startswith("/var/lib/cobbler/snippets"):
-            utils.die(self.logger, "unable to view or edit snippet in this location")
-        
-        if is_read:
-            fileh = open(snippet_file,"r")
-            data = fileh.read()
-            fileh.close()
-            return data
+        try:
+            utils.mkdir(os.path.dirname(file_path))
+        except:
+            utils.die(self.logger, "unable to create directory for kickstart template at %s" % file_path)
+
+        fileh = open(file_path, "w+")
+        fileh.write(data)
+        fileh.close()
+
+        return True
+
+    def remove_kickstart_template_file(self, file_path, token):
+        """
+        Remove a kickstart template file
+
+        @param str file_path kickstart template file path
+        @param ? token
+        @return bool if operation was successful
+        """
+
+        what = "write_kickstart_template"
+        self._log(what, name=file_path, token=token)
+        self.check_access(token, what, file_path, True)
+        self._validate_ks_template_path(file_path)
+
+        if not self.is_kickstart_in_use(file_path, token):
+            os.remove(file_path)
         else:
-            if new_data == -1:
-                # FIXME: no way to check if something is using it
-                os.remove(snippet_file)
-            else:
-                # path_part(a,b) checks for the path b to be inside path a. It is
-                # guaranteed to return either an empty string (meaning b is NOT inside
-                # a), or a path starting with '/'. If the path ends with '/' the sub-path 
-                # is a directory so we don't write to it.
-
-                # FIXME: shouldn't we get snippetdir from the settings?
-                path_part = utils.path_tail("/var/lib/cobbler/snippets",snippet_file)
-                if path_part != "" and path_part[-1] != "/":
-                    try:
-                        utils.mkdir(os.path.dirname(snippet_file))
-                    except:
-                        utils.die(self.logger, "unable to create directory for snippet file: '%s'" % snippet_file)
-                    fileh = open(snippet_file,"w+")
-                    fileh.write(new_data)
-                    fileh.close()
-                else:
-                    utils.die(self.logger, "invalid snippet file specified: '%s'" % snippet_file)
-            return True
+           utils.die(self.logger, "attempt to delete in-use file")
+
+        return True
+
+    # FIXME: duplicated code for kickstart and snippet
+    # FIXME: need to move to API level functions
+
+    def _validate_ks_snippet_path(self, path):
+
+        if path.find("..") != -1 or not path.startswith("/"):
+             utils.die(self.logger, "Invalid kickstart snippet file location %s" % path)
+
+        if not path.startswith(KICKSTART_SNIPPET_BASE_DIR):
+            error = "Invalid kickstart snippet file location %s, it is not inside %s" % (path, KICKSTART_SNIPPET_BASE_DIR)
+            utils.die(self.logger, error)
+
+    def read_kickstart_snippet_file(self, file_path, token):
+        """
+        Read a kickstart snippet file
+
+        @param str file_path kickstart snippet file path
+        @param ? token
+        @return str file content
+        """
+
+        what = "read_kickstart_snippet"
+        self._log(what, name=file_path, token=token)
+        self.check_access(token, what, file_path, True)
+        self._validate_ks_snippet_path(file_path)
+
+        fileh = open(file_path, "r")
+        data = fileh.read()
+        fileh.close()
+
+        return data
+
+    def write_kickstart_snippet_file(self, file_path, data, token):
+        """
+        Write a kickstart snippet file
+
+        @param str file_path kickstart snippet file path
+        @param str data new file content
+        @param ? token
+        @return bool if operation was successful
+     """
+
+        what = "write_kickstart_snippet"
+        self._log(what, name=file_path, token=token)
+        self.check_access(token, what, file_path, True)
+        self._validate_ks_snippet_path(file_path)
+
+        try:
+            utils.mkdir(os.path.dirname(file_path))
+        except:
+            utils.die(self.logger, "unable to create directory for snippet at %s" % file_path)
+
+        fileh = open(file_path, "w+")
+        fileh.write(data)
+        fileh.close()
+
+        return True
+
+    def remove_kickstart_snippet_file(self, file_path, token):
+        """
+        Remove a kickstart snippet file
+
+        @param str file_path kickstart snippet file path
+        @param ? token
+        @return bool if operation was successful
+        """
+
+        what = "write_kickstart_snippet"
+        self._log(what, name=file_path, token=token)
+        self.check_access(token, what, file_path, True)
+        self._validate_ks_snippet_path(file_path)
 
+        # FIXME: could check if snippet is in use
+        snippet_file_path = KICKSTART_SNIPPET_BASE_DIR + file_path
+        os.remove(file_path)
+
+        return True
 
     def power_system(self,object_id,power=None,token=None,logger=None):
         """
diff --git a/web/cobbler_web/views.py b/web/cobbler_web/views.py
index 692ba65..72ceb14 100644
--- a/web/cobbler_web/views.py
+++ b/web/cobbler_web/views.py
@@ -632,178 +632,184 @@ def import_run(request):
 # ======================================================================
 
 def ksfile_list(request, page=None):
-   """
-   List all kickstart templates and link to their edit pages.
-   """
-   if not test_user_authenticated(request): return login(request, next="/cobbler_web/ksfile/list", expired=True)
-   ksfiles = remote.get_kickstart_templates(request.session['token'])
-
-   ksfile_list = []
-   for ksfile in ksfiles:
-      if ksfile.startswith("/var/lib/cobbler/kickstarts") or ksfile.startswith("/etc/cobbler"):
-         ksfile_list.append((ksfile,ksfile.replace('/var/lib/cobbler/kickstarts/',''),'editable'))
-      elif ksfile.startswith("http://") or ksfile.startswith("ftp://"):
-         ksfile_list.append((ksfile,ksfile,'','viewable'))
-      else:
-         ksfile_list.append((ksfile,ksfile,None))
+    """
+    List all kickstart templates and link to their edit pages.
+    """
+    if not test_user_authenticated(request):
+        return login(request, next="/cobbler_web/ksfile/list", expired=True)
+    ksfiles = remote.get_kickstart_templates(request.session['token'])
+
+    ksfile_list = []
+    base_dir = "/var/lib/cobbler/kickstarts/"
+    for ksfile in ksfiles:
+        if ksfile.startswith(base_dir):
+            ksfile_list.append((ksfile, ksfile.replace(base_dir, ''), 'editable'))
+        else:
+            return error_page(request, "Invalid kickstart template at %s, outside %s" % (ksfile, base_dir))
 
-   t = get_template('ksfile_list.tmpl')
-   html = t.render(RequestContext(request,{
-       'what':'ksfile',
-       'ksfiles': ksfile_list,
-       'version': remote.extended_version(request.session['token'])['version'],
-       'username': username,
-       'item_count': len(ksfile_list[0]),
-   }))
-   return HttpResponse(html)
+    t = get_template('ksfile_list.tmpl')
+    html = t.render(RequestContext(request, {
+        'what': 'ksfile',
+        'ksfiles': ksfile_list,
+        'version': remote.extended_version(request.session['token'])['version'],
+        'username': username,
+        'item_count': len(ksfile_list[0]),
+    }))
+    return HttpResponse(html)
 
 # ======================================================================
 
 @csrf_protect
 def ksfile_edit(request, ksfile_name=None, editmode='edit'):
-   """
-   This is the page where a kickstart file is edited.
-   """
-   if not test_user_authenticated(request): return login(request, next="/cobbler_web/ksfile/edit/file:%s" % ksfile_name, expired=True)
-   if editmode == 'edit':
-      editable = False
-   else:
-      editable = True
-   deleteable = False
-   ksdata = ""
-   if not ksfile_name is None:
-      editable = remote.check_access_no_fail(request.session['token'], "modify_kickstart", ksfile_name)
-      deleteable = not remote.is_kickstart_in_use(ksfile_name, request.session['token'])
-      ksdata = remote.read_or_write_kickstart_template(ksfile_name, True, "", request.session['token'])
-
-   t = get_template('ksfile_edit.tmpl')
-   html = t.render(RequestContext(request,{
-       'ksfile_name' : ksfile_name,
-       'deleteable'  : deleteable,
-       'ksdata'      : ksdata,
-       'editable'    : editable,
-       'editmode'    : editmode,
-       'version'     : remote.extended_version(request.session['token'])['version'],
-       'username'    : username
-   }))
-   return HttpResponse(html)
+    """
+    This is the page where a kickstart file is edited.
+    """
+    if not test_user_authenticated(request):
+        return login(request, next="/cobbler_web/ksfile/edit/file:%s" % ksfile_name, expired=True)
+    if editmode == 'edit':
+        editable = False
+    else:
+        editable = True
+    deleteable = False
+    ksdata = ""
+    if not ksfile_name is None:
+        editable = remote.check_access_no_fail(request.session['token'], "modify_kickstart", ksfile_name)
+        deleteable = not remote.is_kickstart_in_use(ksfile_name, request.session['token'])
+        ksdata = remote.read_kickstart_template(ksfile_name, request.session['token'])
+
+    t = get_template('ksfile_edit.tmpl')
+    html = t.render(RequestContext(request, {
+        'ksfile_name': ksfile_name,
+        'deleteable': deleteable,
+        'ksdata': ksdata,
+        'editable': editable,
+        'editmode': editmode,
+        'version': remote.extended_version(request.session['token'])['version'],
+        'username': username
+    }))
+    return HttpResponse(html)
 
 # ======================================================================
 
 @require_POST
 @csrf_protect
 def ksfile_save(request):
-   """
-   This page processes and saves edits to a kickstart file.
-   """
-   if not test_user_authenticated(request): return login(request, next="/cobbler_web/ksfile/list", expired=True)
-   # FIXME: error checking
+    """
+    This page processes and saves edits to a kickstart file.
+    """
+    if not test_user_authenticated(request):
+        return login(request, next="/cobbler_web/ksfile/list", expired=True)
+    # FIXME: error checking
 
-   editmode = request.POST.get('editmode', 'edit')
-   ksfile_name = request.POST.get('ksfile_name', None)
-   ksdata = request.POST.get('ksdata', "").replace('\r\n','\n')
+    editmode = request.POST.get('editmode', 'edit')
+    ksfile_name = request.POST.get('ksfile_name', None)
+    ksdata = request.POST.get('ksdata', "").replace('\r\n', '\n')
 
-   if ksfile_name == None:
-      return HttpResponse("NO KSFILE NAME SPECIFIED")
-   if editmode != 'edit':
-      ksfile_name = "/var/lib/cobbler/kickstarts/" + ksfile_name
+    if ksfile_name == None:
+        return HttpResponse("NO KSFILE NAME SPECIFIED")
+    if editmode != 'edit':
+        ksfile_name = "/var/lib/cobbler/kickstarts/" + ksfile_name
 
-   delete1   = request.POST.get('delete1', None)
-   delete2   = request.POST.get('delete2', None)
+    delete1 = request.POST.get('delete1', None)
+    delete2 = request.POST.get('delete2', None)
 
-   if delete1 and delete2:
-      remote.read_or_write_kickstart_template(ksfile_name, False, -1, request.session['token'])
-      return HttpResponseRedirect('/cobbler_web/ksfile/list')
-   else:
-      remote.read_or_write_kickstart_template(ksfile_name,False,ksdata,request.session['token'])
-      return HttpResponseRedirect('/cobbler_web/ksfile/edit/file:%s' % ksfile_name)
+    if delete1 and delete2:
+        remote.remove_kickstart_template(ksfile_name, request.session['token'])
+        return HttpResponseRedirect('/cobbler_web/ksfile/list')
+    else:
+        remote.write_kickstart_template(ksfile_name, ksdata, request.session['token'])
+        return HttpResponseRedirect('/cobbler_web/ksfile/list')
 
 # ======================================================================
 
 def snippet_list(request, page=None):
-   """
-   This page lists all available snippets and has links to edit them.
-   """
-   if not test_user_authenticated(request): return login(request, next="/cobbler_web/snippet/list", expired=True)
-   snippets = remote.get_snippets(request.session['token'])
-
-   snippet_list = []
-   for snippet in snippets:
-      if snippet.startswith("/var/lib/cobbler/snippets"):
-         snippet_list.append((snippet,snippet.replace("/var/lib/cobbler/snippets/",""),'editable'))
-      else:
-         snippet_list.append((snippet,snippet,None))
-
-   t = get_template('snippet_list.tmpl')
-   html = t.render(RequestContext(request,{
-       'what'     : 'snippet',
-       'snippets' : snippet_list,
-       'version'  : remote.extended_version(request.session['token'])['version'],
-       'username' : username
-   }))
-   return HttpResponse(html)
+    """
+    This page lists all available snippets and has links to edit them.
+    """
+    if not test_user_authenticated(request):
+        return login(request, next="/cobbler_web/snippet/list", expired=True)
+    snippets = remote.get_snippets(request.session['token'])
+
+    snippet_list = []
+    base_dir = "/var/lib/cobbler/snippets/"
+    for snippet in snippets:
+        if snippet.startswith(base_dir):
+            snippet_list.append((snippet, snippet.replace(base_dir, ""), 'editable'))
+        else:
+            return error_page(request, "Invalid snippet at %s, outside %s" % (snippet, base_dir))
+
+    t = get_template('snippet_list.tmpl')
+    html = t.render(RequestContext(request, {
+        'what': 'snippet',
+        'snippets': snippet_list,
+        'version': remote.extended_version(request.session['token'])['version'],
+        'username': username
+    }))
+    return HttpResponse(html)
 
 # ======================================================================
 
 @csrf_protect
 def snippet_edit(request, snippet_name=None, editmode='edit'):
-   """
-   This page edits a specific snippet.
-   """
-   if not test_user_authenticated(request): return login(request, next="/cobbler_web/edit/file:%s" % snippet_name, expired=True)
-   if editmode == 'edit':
-      editable = False
-   else:
-      editable = True
-   deleteable = False
-   snippetdata = ""
-   if not snippet_name is None:
-      editable = remote.check_access_no_fail(request.session['token'], "modify_snippet", snippet_name)
-      deleteable = True
-      snippetdata = remote.read_or_write_snippet(snippet_name, True, "", request.session['token'])
-
-   t = get_template('snippet_edit.tmpl')
-   html = t.render(RequestContext(request,{
-       'snippet_name' : snippet_name,
-       'deleteable'   : deleteable,
-       'snippetdata'  : snippetdata,
-       'editable'     : editable,
-       'editmode'     : editmode,
-       'version'      : remote.extended_version(request.session['token'])['version'],
-       'username'     : username
-   }))
-   return HttpResponse(html)
+    """
+    This page edits a specific snippet.
+    """
+    if not test_user_authenticated(request):
+        return login(request, next="/cobbler_web/edit/file:%s" % snippet_name, expired=True)
+    if editmode == 'edit':
+        editable = False
+    else:
+        editable = True
+    deleteable = False
+    snippetdata = ""
+    if not snippet_name is None:
+        editable = remote.check_access_no_fail(request.session['token'], "modify_snippet", snippet_name)
+        deleteable = True
+        snippetdata = remote.read_kickstart_snippet(snippet_name, request.session['token'])
+
+    t = get_template('snippet_edit.tmpl')
+    html = t.render(RequestContext(request, {
+        'snippet_name': snippet_name,
+        'deleteable': deleteable,
+        'snippetdata': snippetdata,
+        'editable': editable,
+        'editmode': editmode,
+        'version': remote.extended_version(request.session['token'])['version'],
+        'username': username
+    }))
+    return HttpResponse(html)
 
 # ======================================================================
 
 @require_POST
 @csrf_protect
 def snippet_save(request):
-   """
-   This snippet saves a snippet once edited.
-   """
-   if not test_user_authenticated(request): return login(request, next="/cobbler_web/snippet/list", expired=True)
-   # FIXME: error checking
-
-   editmode = request.POST.get('editmode', 'edit')
-   snippet_name = request.POST.get('snippet_name', None)
-   snippetdata = request.POST.get('snippetdata', "").replace('\r\n','\n')
-
-   if snippet_name == None:
-      return HttpResponse("NO SNIPPET NAME SPECIFIED")
-   if editmode != 'edit':
-      if snippet_name.find("/var/lib/cobbler/snippets/") != 0:
-          snippet_name = "/var/lib/cobbler/snippets/" + snippet_name
-
-   delete1   = request.POST.get('delete1', None)
-   delete2   = request.POST.get('delete2', None)
-
-   if delete1 and delete2:
-      remote.read_or_write_snippet(snippet_name, False, -1, request.session['token'])
-      return HttpResponseRedirect('/cobbler_web/snippet/list')
-   else:
-      remote.read_or_write_snippet(snippet_name,False,snippetdata,request.session['token'])
-      return HttpResponseRedirect('/cobbler_web/snippet/edit/file:%s' % snippet_name)
+    """
+    This snippet saves a snippet once edited.
+    """
+    if not test_user_authenticated(request):
+        return login(request, next="/cobbler_web/snippet/list", expired=True)
+    # FIXME: error checking
+
+    editmode = request.POST.get('editmode', 'edit')
+    snippet_name = request.POST.get('snippet_name', None)
+    snippetdata = request.POST.get('snippetdata', "").replace('\r\n', '\n')
+
+    if snippet_name == None:
+        return HttpResponse("NO SNIPPET NAME SPECIFIED")
+    if editmode != 'edit':
+        if snippet_name.find("/var/lib/cobbler/snippets/") != 0:
+            snippet_name = "/var/lib/cobbler/snippets/" + snippet_name
+
+    delete1 = request.POST.get('delete1', None)
+    delete2 = request.POST.get('delete2', None)
+
+    if delete1 and delete2:
+        remote.remove_kickstart_snippet(snippet_name, request.session['token'])
+        return HttpResponseRedirect('/cobbler_web/snippet/list')
+    else:
+        remote.write_kickstart_snippet(snippet_name, snippetdata, request.session['token'])
+        return HttpResponseRedirect('/cobbler_web/snippet/list')
 
 # ======================================================================
 
-- 
1.8.4.5

Places

File bnc_877009.patch of Package cobbler.208

Places