Repairman 2

taskManager/settings.py

@@ -36,6 +36,7 @@
     'django.contrib.admin',
     'django.contrib.auth',
     'django.contrib.contenttypes',
+    'django.contrib.sessions',
     'django.contrib.messages',
     'django.contrib.staticfiles',
     'taskManager'
@@ -83,6 +84,9 @@
 
 MESSAGE_STORAGE = 'django.contrib.messages.storage.cookie.CookieStorage'
 
+# leaving here because I think I'll get back to it:
+# MESSAGE_STORAGE = 'django.contrib.messages.storage.cookie.SessionStorage'
+
 # Static files (CSS, JavaScript, Images)
 # https://docs.djangoproject.com/en/1.7/howto/static-files/
 
@@ -98,7 +102,7 @@
 LOGIN_URL = '/taskManager/login/'
 
 # A6: Sensitive Data Exposure
-PASSWORD_HASHERS = ['django.contrib.auth.hashers.MD5PasswordHasher']
+PASSWORD_HASHERS = ['django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher']
 
 # A2: Broken Auth and Session Management
 SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies"

taskManager/templates/taskManager/upload.html

@@ -11,11 +11,11 @@
             <div class="panel-body">
                 <form method="post" role="form" enctype="multipart/form-data">
                     {% csrf_token %}
-                    <div class="form-group">
+ <!--                    <div class="form-group">
                         <label>File Name</label>
                         <input id="id_name" maxlength="300" name="name" type="text" />
                     </div>
-                    <div class="form-group">
+  -->                   <div class="form-group">
                         <input id="id_file" name="file" type="file" />
                     </div>
                     <button type="submit" class="btn btn-info">Upload</button>

taskManager/views.py

@@ -178,18 +178,18 @@ def upload(request, project_id):
             name = request.POST.get('name', False)
             upload_path = store_uploaded_file(name, request.FILES['file'])
 
-            #A1 - Injection (SQLi)
-            curs = connection.cursor()
-            curs.execute(
-                "insert into taskManager_file ('name','path','project_id') values ('%s','%s',%s)" %
-                (name, upload_path, project_id))
-
-            # file = File(
-            #name = name,
-            #path = upload_path,
-            # project = proj)
-
-            # file.save()
+            # #A1 - Injection (SQLi)
+            # curs = connection.cursor()
+            # curs.execute(
+            #     "insert into taskManager_file ('name','path','project_id') values ('%s','%s',%s)" %
+            #     (name, upload_path, project_id))
+
+            file = File(
+            name = name,
+            path = upload_path,
+            project = proj)
+
+            file.save()
 
             return redirect('/taskManager/' + project_id +
                             '/', {'new_file_added': True})

vulnerability-report.md

@@ -0,0 +1,175 @@
+#Vulnerability Report
+Reviewer 1: Ted Callahan
+Reviewer 2: Rick Valenzuela
+
+Date: February 1st, 2017
+
+##Reviewing nVisium Task Manager
+
+
+##Vulnerability A1: Injection
+###Exposure
+We found an instance of an SQL Injection vulnerability in the file upload form (line 183 in views.py).
+
+By exploiting this problem, we are able to perform SQL injection and retrieve data or drop tables from the database.
+
+###Repair
+Rather than use a formatted SQL command with input from the form, the site should use the Django ORM to create a File object instance for addition to the databse.
+
+Problem Code (Lines 182 - 185, views.py):
+```
+curs = connection.cursor()
+curs.execute(
+    "insert into taskManager_file ('name','path','project_id') values ('%s','%s',%s)" %
+    (name, upload_path, project_id))
+```
+
+###Solution:
+```
+file = File(
+name = name,
+path = upload_path,
+project = proj)
+
+file.save()
+```
+
+##Vulnerability A2: Broken Authentication and Sesson Management
+###Exposure
+Various upload functions call the store_uploaded_file() funtion at line 24 of misc.py.  This function passes a given filename to the os.system function giving a user the potential to pass in shell commands as a filename.
+
+###Repair
+Do not allow the user to pass in a filename.  Give the file a randomly generated name; there is no reason for a user to give a file a name for use in the application.
+
+Instead, create a File instance and save the file instance in the database.  The user can give the File object a name, thus preventing any need for calling os.system or any commands that rely on command line commands.
+
+###Solution:
+Comment out or remove lines 14-17 of templates/taskManager/upload.html:
+```
+<div class="form-group">
+    <label>File Name</label>
+    <input id="id_name" maxlength="300" name="name" type="text" />
+</div>
+```
+
+##Vulnerability A3: Cross-Site Scripting (XSS)
+###Exposure
+The User registration form (class UserForm) as defined in forms.py (lines 71 - 75) is insecure because it does not explicitly state which fields are allowed.  A user could add fields for other attributes of the Django user model (in this instance, is_superuser and is_staff).
+
+By exploiting this problem, we are able to add form fields to the User registration form to edit user attributes on the Django model.
+
+###Repair
+Instead of a partial exclude list of fields, use the fields attribute in the Meta class to explicitly declare which fields should be in the form.
+
+Problem Code (line 75 in forms.py):
+```
+exclude = ['groups', 'user_permissions', 'last_login', 'date_joined', 'is_active']
+```
+
+###Solution:
+```
+fields = ['username', 'email', 'first_name', 'last_name', 'password']
+```
+
+##Vulnerability A4: Insecure Direct Object References
+###Exposure
+Users are not authenticated for pages that should require it; therefore, a user's data can be accessed, deleted or changed by others simply if another user feeds in the proper parameters to hit that page. Several views in views.py for a CRUD operation lacked an authentication check. These are the view functions that begin on lines: 112, 170, 221, 243, 277, 304, 329, 358, 520, 541, 567 and 719.
+
+By exploiting this problem, we were able to modify, delete and create tasks and files on another user's account.
+
+###Repair
+At the start of each view function, add an authentication check.
+
+###Solution
+add this to each function:
+```
+user = request.user
+if user.is_authenticated():
+```
+
+
+##Vulnerability A5: Security Misconfiguration
+Misconfiguration can give too much information to malicious users.  By leaving DEBUG mode as True in production, malicious users are presented with information that can be used to potentially access or manipulate unsecured parts of the site.
+
+###Repair
+Problem Code: (Lines 24, 28-29, 64-69) in settings.py
+```
+SECRET_KEY = '0yxzudryd8)-%)(fz&7q-!v&cq1u6vbfoc4u7@u_&i)b@4eh^q'
+DEBUG = True
+TEMPLATE_DEBUG = True
+DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
+    }
+}
+```
+
+###Solution:
+Set DEBUG and TEMPLATE DEBUG to False for production environments.  Additionally, use environmental variables to store items such as the SECRET_KEY and DATABASE attributes (and even the DEBUG statements).
+
+```
+DEBUG = os.environ.get("DEBUG", "False")
+TEMPLATE_DEBUG = os.environ.get("DEBUG", "False")
+SECRET_KEY = os.environ["SECRET_KEY"]
+DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': os.environ.get("DB_NAME", "")
+    }
+}
+```
+
+##Vulnerabilty A6: Sensitive Data Exposure
+###Exposure
+Using a weak hashing algorithm for passwords makes it easier than it should be for hackers to break encryption.
+
+###Repair
+Problem Code (Line 101 in settings.py)
+```
+PASSWORD_HASHERS = ['django.contrib.auth.hashers.MD5PasswordHasher']
+```
+
+###Solution
+Instead of using MD5 for hashing passwords, use a more secure hash such as PBKDF2SHA1 or BCrypt
+
+```
+PASSWORD_HASHERS = ['django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher']
+```
+
+##Vulnerability A7: Missing Function Level Access Control
+###Exposure
+Access.  Any views or operations that result in a CRUD operation or display of potentially sensitive data should have a check to ensure that an authenticated user has the appropriate permissions.
+
+###Repair
+Problem Code: Lines 120 - 153 in views.py
+When handling the POST request, there is no check for users permissions (which is checked in the following get request.)
+
+###Solution
+This can be solved with an explicit check for permission inside the function, or with a more general solution implementing permissions decorators (for function views) or mixins (for class based views). See Vulnerability 7 for details on using the mixins and decorators.  Alternatively, an explicit check can be used to verify permissions stored on the user object in the view.
+
+if user.has_perm('can_change_group'):
+
+To use this properly, the site must be set up to assign appropriate permissions and users when new users are created.
+
+To define permissions and groups the user can create new Model permissions by declaring them in the Models as such:
+class ExampleModel(models.Model):
+    class Meta:
+        permissions = (
+            ("view_model", "Can view model")
+        )
+
+Then, when creating Users, add the permission to that instance of the user model or make it a part of a model tied to the user model (e.g. a Profile model.)
+
+##Vulnerability 8: Cross-Site Request Forgery (CSRF)
+###Exposure:
+CSRF Protection
+The @csrf_exempt decorator is applied to several views that include forms.  Ideally, this exemption decorator should only be applied to the login form and @csrf protection should exist for all instances where a user can submit data.
+
+###Repair:
+Problem Code: lines 710, 813
+
+###Solution:
+Remove the @csrf_exempt decorator from these functions.  Ensure that
+django.middleware.csrf.CsrfViewMiddleware is included in the MIDDLEWARE_CLASSES list of settings.py. 
+