From 1938f178351fd046346c0f99845855a5c6a09345 Mon Sep 17 00:00:00 2001 From: Ted Callahan Date: Fri, 3 Feb 2017 16:09:38 -0800 Subject: [PATCH 1/8] Added vulnerability report. --- vulnerabilityreport.md | 47 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 vulnerabilityreport.md diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md new file mode 100644 index 00000000..7528e82c --- /dev/null +++ b/vulnerabilityreport.md @@ -0,0 +1,47 @@ +Vulnerability Report +Reviewer 1: Ted Callahan + +Reviewer 2: Rick Valenzuela + +Date: February 1st, 2017 + +Reviewing nVisium Task Manager + +Vulnerability 1 +Exposure +We found an instance of [vulnerability 1] by typing some relevant code into some vulnerable field OR by doing some edge-case thing. + +By exploiting [this vulnerability], we were able to retrieve XYZ attributes from the site / access to some unauthorized part of the site / something else valuable. + +Repair +problem_file1.py and problem_file2.py contained the vulnerability. We were able to fix the first with the following adjustment(s): + +problem_file1.py + +Some body of relevant code that solves our problem +problem_file2.py + + +Vulnerability 1 +Exposure +We found an instance of 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() + From 6aba55c194eccf90d1e6d16e681f1f625a24e6ef Mon Sep 17 00:00:00 2001 From: Ted Callahan Date: Fri, 3 Feb 2017 17:11:13 -0800 Subject: [PATCH 2/8] vulnerability report completed for security issues 1 - 4 --- vulnerabilityreport.md | 89 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md index 7528e82c..5551858b 100644 --- a/vulnerabilityreport.md +++ b/vulnerabilityreport.md @@ -45,3 +45,92 @@ project = proj) file.save() +Vulnerability 2 +Exposure +Various upload functions call the store_uploaded_file() funtion at line 24 of msc.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 file name. 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. + + +Vulnerability 3 +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 (such as is_superuser). + +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 4 +Something to do with: +SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies" + + +Vulnerability 5 +Exposure +The template base_backend.html inserts the username with a safe flag. This allows the potential for a JS script as a username to be rendered to the page, causing it to be run because it will not be escaped. + +Repair +Instead of declaring safe output with the safe flag, do not do that. + +Problem Code (line 58 of base_backend.html): + {{user.username|safe}} + +Solution: + {{user.username}} + + +Vulnerabilty 6 +Exposure +Website is generally at-risk for Cross Site Scripting attacks. By turning on Django's SECURE_BROWSER_XSS_FILTER setting, Django will add X-XSS-Protection to all responses. The browser will then automatically block all XSS attacks it can detect. + +Repair +Set SECURE_BROWSER_XSS_FILTER = True in settings.py + + +Vulnerability 7 +Exposure +All CRUD operations are vulnerable to Insecure Direct Object Reference (IDOR). Basically, in order to prevent unauthorized users or users with inappropriate permissions, the site should require all proper credentials and permissions from an user before a CRUD operation is performed. + +For Class Based Views, use the LoginReqiuredMixin and PermissionRequiredMixin + +Repair +Apply @login_required and @permission_required decorators for any function in views.py that performs a CRUD operation. + +Problem Code functions listed from views.py: + - manage_products + - manage_groups + - upload + - task_create + - task_edit + - task_delete + - project_delete + - project_edit + - project_create + - note_create + - note_edit + - note_delete + +Solution: +### For Function Views: +from django.contrib.auth.decorators import permission_required, login_required + +@login_required +@permission_required +def function_view_with_CRUD(request): + + +### For Class Based Views: +from django.contrib.auth.mixins import PermissionRequiredMixin, LoginRequiredMixin + +class ClassBasedCRUDView(LoginRequiredMixin, PermissionRequiredMixin, ClassView): From b50f8a504ef9b849f6359e5de9181f800237aefa Mon Sep 17 00:00:00 2001 From: Ted Callahan Date: Mon, 6 Feb 2017 17:49:12 -0800 Subject: [PATCH 3/8] Completed initial review for security flaws 5 - 8. --- taskManager/settings.py | 2 +- taskManager/views.py | 24 ++++++------- vulnerabilityreport.md | 75 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 13 deletions(-) diff --git a/taskManager/settings.py b/taskManager/settings.py index 17bb6978..97f661ff 100644 --- a/taskManager/settings.py +++ b/taskManager/settings.py @@ -98,7 +98,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" diff --git a/taskManager/views.py b/taskManager/views.py index 274de347..49e8f8d8 100644 --- a/taskManager/views.py +++ b/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}) diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md index 5551858b..93bab9f7 100644 --- a/vulnerabilityreport.md +++ b/vulnerabilityreport.md @@ -134,3 +134,78 @@ def function_view_with_CRUD(request): from django.contrib.auth.mixins import PermissionRequiredMixin, LoginRequiredMixin class ClassBasedCRUDView(LoginRequiredMixin, PermissionRequiredMixin, ClassView): + +Vulnerability 8 +Exposure +Misconfiguration can give too much information to malicious users. By leaving DEBUG modes True in production, malicious users are presented with information that can be used to potentially access or manipulate unsecured parts of the site. + +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", "") + } +} + +Vulnerability 9 +Exposure +Using a weak hashing algorithm for passwords makes it easier than it should be for hackers to break encryption. + +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 10 +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. + +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 11: +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. + +Problem Code(line 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. + From 54cff8cf18e7bbabc482957feb8203e9836756f6 Mon Sep 17 00:00:00 2001 From: Rick Valenzuela Date: Mon, 6 Feb 2017 20:08:03 -0800 Subject: [PATCH 4/8] added solution info to vuln 2, made edits, formatting --- manage.py | 0 taskManager/templates/taskManager/upload.html | 4 +- vulnerabilityreport.md | 52 +++++++------------ 3 files changed, 22 insertions(+), 34 deletions(-) mode change 100644 => 100755 manage.py diff --git a/manage.py b/manage.py old mode 100644 new mode 100755 diff --git a/taskManager/templates/taskManager/upload.html b/taskManager/templates/taskManager/upload.html index fb93b38a..57ceec4a 100644 --- a/taskManager/templates/taskManager/upload.html +++ b/taskManager/templates/taskManager/upload.html @@ -11,11 +11,11 @@
{% csrf_token %} -
+
diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md index 93bab9f7..e17d98a8 100644 --- a/vulnerabilityreport.md +++ b/vulnerabilityreport.md @@ -1,56 +1,44 @@ -Vulnerability Report -Reviewer 1: Ted Callahan +#Vulnerability Report +###Reviewer 1: Ted Callahan +###Reviewer 2: Rick Valenzuela -Reviewer 2: Rick Valenzuela +###Date: February 1st, 2017 -Date: February 1st, 2017 +##Reviewing nVisium Task Manager -Reviewing nVisium Task Manager - -Vulnerability 1 -Exposure -We found an instance of [vulnerability 1] by typing some relevant code into some vulnerable field OR by doing some edge-case thing. - -By exploiting [this vulnerability], we were able to retrieve XYZ attributes from the site / access to some unauthorized part of the site / something else valuable. - -Repair -problem_file1.py and problem_file2.py contained the vulnerability. We were able to fix the first with the following adjustment(s): - -problem_file1.py - -Some body of relevant code that solves our problem -problem_file2.py - - -Vulnerability 1 -Exposure -We found an instance of SQL Injection vulnerability in the file upload form (line 183 in views.py). +##Vulnerability 1 +###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. +###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: +###Solution: +``` file = File( name = name, path = upload_path, project = proj) file.save() +``` -Vulnerability 2 -Exposure -Various upload functions call the store_uploaded_file() funtion at line 24 of msc.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. +##Vulnerability 2 +###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 file name. 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. +###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. From bd8b8be08fe2cc65bc7284eea595712de94d1986 Mon Sep 17 00:00:00 2001 From: Rick Valenzuela Date: Mon, 6 Feb 2017 20:55:27 -0800 Subject: [PATCH 5/8] formatted report for markdown, small edits --- taskManager/settings.py | 4 ++ vulnerabilityreport.md | 99 ++++++++++++++++++++++++++--------------- 2 files changed, 66 insertions(+), 37 deletions(-) diff --git a/taskManager/settings.py b/taskManager/settings.py index 97f661ff..20549efe 100644 --- a/taskManager/settings.py +++ b/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/ diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md index e17d98a8..3ed3c651 100644 --- a/vulnerabilityreport.md +++ b/vulnerabilityreport.md @@ -42,57 +42,71 @@ Do not allow the user to pass in a filename. Give the file a randomly generated 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: +``` +
+ + +
+``` -Vulnerability 3 -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 (such as is_superuser). +##Vulnerability 3 +###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 +###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: +###Solution: +``` fields = ['username', 'email', 'first_name', 'last_name', 'password'] +``` - -Vulnerability 4 +##Vulnerability 4 Something to do with: SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies" -Vulnerability 5 -Exposure +##Vulnerability 5 +###Exposure The template base_backend.html inserts the username with a safe flag. This allows the potential for a JS script as a username to be rendered to the page, causing it to be run because it will not be escaped. -Repair +###Repair Instead of declaring safe output with the safe flag, do not do that. Problem Code (line 58 of base_backend.html): +``` {{user.username|safe}} +``` Solution: +``` {{user.username}} +``` +##Vulnerabilty 6 +###Exposure +Website is generally at-risk for Cross-Site Scripting attacks. By turning on Django's SECURE_BROWSER_XSS_FILTER setting, Django will add X-XSS-Protection to all responses. The browser will then automatically block all XSS attacks it can detect. -Vulnerabilty 6 -Exposure -Website is generally at-risk for Cross Site Scripting attacks. By turning on Django's SECURE_BROWSER_XSS_FILTER setting, Django will add X-XSS-Protection to all responses. The browser will then automatically block all XSS attacks it can detect. - -Repair +###Repair Set SECURE_BROWSER_XSS_FILTER = True in settings.py -Vulnerability 7 -Exposure +##Vulnerability 7 +###Exposure All CRUD operations are vulnerable to Insecure Direct Object Reference (IDOR). Basically, in order to prevent unauthorized users or users with inappropriate permissions, the site should require all proper credentials and permissions from an user before a CRUD operation is performed. For Class Based Views, use the LoginReqiuredMixin and PermissionRequiredMixin -Repair +###Repair Apply @login_required and @permission_required decorators for any function in views.py that performs a CRUD operation. Problem Code functions listed from views.py: @@ -109,25 +123,29 @@ Problem Code functions listed from views.py: - note_edit - note_delete -Solution: -### For Function Views: +###Solution: +####For Function Views: +``` from django.contrib.auth.decorators import permission_required, login_required @login_required @permission_required def function_view_with_CRUD(request): +``` - -### For Class Based Views: +####For Class Based Views: +``` from django.contrib.auth.mixins import PermissionRequiredMixin, LoginRequiredMixin class ClassBasedCRUDView(LoginRequiredMixin, PermissionRequiredMixin, ClassView): +``` -Vulnerability 8 -Exposure -Misconfiguration can give too much information to malicious users. By leaving DEBUG modes True in production, malicious users are presented with information that can be used to potentially access or manipulate unsecured parts of the site. +##Vulnerability 8 +###Exposure +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. -Problem Code (Lines 24, 28-29, 64-69) in settings.py +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 @@ -137,10 +155,12 @@ DATABASES = { 'NAME': os.path.join(BASE_DIR, 'db.sqlite3'), } } +``` -Solution: +###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"] @@ -150,28 +170,32 @@ DATABASES = { 'NAME': os.environ.get("DB_NAME", "") } } +``` -Vulnerability 9 -Exposure +##Vulnerability 9 +###Exposure Using a weak hashing algorithm for passwords makes it easier than it should be for hackers to break encryption. Problem Code (Line 101 in settings.py) +``` PASSWORD_HASHERS = ['django.contrib.auth.hashers.MD5PasswordHasher'] +``` -Solution +###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 10 -Exposure +##Vulnerability 10 +###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. -Problem Code: -Lines 120 - 153 in views.py +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'): @@ -187,13 +211,14 @@ class ExampleModel(models.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 11: +##Vulnerability 11: +###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. Problem Code(line 710, 813) -Solution: +###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. From 1fc467a6ea45dd20fb35a7ccb98faf5a7a6fad7c Mon Sep 17 00:00:00 2001 From: Rick Valenzuela Date: Tue, 21 Feb 2017 18:43:37 -0800 Subject: [PATCH 6/8] reformatted some markdown --- vulnerabilityreport.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md index 3ed3c651..628ae0ec 100644 --- a/vulnerabilityreport.md +++ b/vulnerabilityreport.md @@ -1,8 +1,8 @@ #Vulnerability Report -###Reviewer 1: Ted Callahan -###Reviewer 2: Rick Valenzuela +Reviewer 1: Ted Callahan +Reviewer 2: Rick Valenzuela -###Date: February 1st, 2017 +Date: February 1st, 2017 ##Reviewing nVisium Task Manager From 592e0cdbb7ac842f57f422e26a3040e583f25e0e Mon Sep 17 00:00:00 2001 From: Rick Valenzuela Date: Tue, 21 Feb 2017 22:20:24 -0800 Subject: [PATCH 7/8] Edited flow/structure to follow OWASP 1-8 of top 10 --- vulnerabilityreport.md | 85 ++++++++---------------------------------- 1 file changed, 16 insertions(+), 69 deletions(-) diff --git a/vulnerabilityreport.md b/vulnerabilityreport.md index 628ae0ec..f28a63e3 100644 --- a/vulnerabilityreport.md +++ b/vulnerabilityreport.md @@ -6,7 +6,8 @@ Date: February 1st, 2017 ##Reviewing nVisium Task Manager -##Vulnerability 1 + +##Vulnerability A1: Injection ###Exposure We found an instance of an SQL Injection vulnerability in the file upload form (line 183 in views.py). @@ -33,7 +34,7 @@ project = proj) file.save() ``` -##Vulnerability 2 +##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. @@ -51,7 +52,7 @@ Comment out or remove lines 14-17 of templates/taskManager/upload.html:
``` -##Vulnerability 3 +##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). @@ -70,78 +71,24 @@ exclude = ['groups', 'user_permissions', 'last_login', 'date_joined', 'is_active fields = ['username', 'email', 'first_name', 'last_name', 'password'] ``` -##Vulnerability 4 -Something to do with: -SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies" - - -##Vulnerability 5 +##Vulnerability A4: Insecure Direct Object References ###Exposure -The template base_backend.html inserts the username with a safe flag. This allows the potential for a JS script as a username to be rendered to the page, causing it to be run because it will not be escaped. +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. -###Repair -Instead of declaring safe output with the safe flag, do not do that. - -Problem Code (line 58 of base_backend.html): -``` - {{user.username|safe}} -``` - -Solution: -``` - {{user.username}} -``` - -##Vulnerabilty 6 -###Exposure -Website is generally at-risk for Cross-Site Scripting attacks. By turning on Django's SECURE_BROWSER_XSS_FILTER setting, Django will add X-XSS-Protection to all responses. The browser will then automatically block all XSS attacks it can detect. +By exploiting this problem, we were able to modify, delete and create tasks and files on another user's account. ###Repair -Set SECURE_BROWSER_XSS_FILTER = True in settings.py - +At the start of each view function, add an authentication check. -##Vulnerability 7 -###Exposure -All CRUD operations are vulnerable to Insecure Direct Object Reference (IDOR). Basically, in order to prevent unauthorized users or users with inappropriate permissions, the site should require all proper credentials and permissions from an user before a CRUD operation is performed. - -For Class Based Views, use the LoginReqiuredMixin and PermissionRequiredMixin - -###Repair -Apply @login_required and @permission_required decorators for any function in views.py that performs a CRUD operation. - -Problem Code functions listed from views.py: - - manage_products - - manage_groups - - upload - - task_create - - task_edit - - task_delete - - project_delete - - project_edit - - project_create - - note_create - - note_edit - - note_delete - -###Solution: -####For Function Views: -``` -from django.contrib.auth.decorators import permission_required, login_required - -@login_required -@permission_required -def function_view_with_CRUD(request): +###Solution +add this to each function: ``` - -####For Class Based Views: +user = request.user +if user.is_authenticated(): ``` -from django.contrib.auth.mixins import PermissionRequiredMixin, LoginRequiredMixin -class ClassBasedCRUDView(LoginRequiredMixin, PermissionRequiredMixin, ClassView): -``` -##Vulnerability 8 -###Exposure +##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. Problem Code: (Lines 24, 28-29, 64-69) in settings.py @@ -172,7 +119,7 @@ DATABASES = { } ``` -##Vulnerability 9 +##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. @@ -188,7 +135,7 @@ Instead of using MD5 for hashing passwords, use a more secure hash such as PBKDF PASSWORD_HASHERS = ['django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher'] ``` -##Vulnerability 10 +##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. @@ -211,7 +158,7 @@ class ExampleModel(models.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 11: +##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. From 3a5ac1ab8944912649e08d934b38e1fbfa51142e Mon Sep 17 00:00:00 2001 From: Rick Valenzuela Date: Tue, 21 Feb 2017 22:23:45 -0800 Subject: [PATCH 8/8] edits for consistency; renamed file to specs --- vulnerabilityreport.md => vulnerability-report.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) rename vulnerabilityreport.md => vulnerability-report.md (98%) diff --git a/vulnerabilityreport.md b/vulnerability-report.md similarity index 98% rename from vulnerabilityreport.md rename to vulnerability-report.md index f28a63e3..7c49b230 100644 --- a/vulnerabilityreport.md +++ b/vulnerability-report.md @@ -91,6 +91,7 @@ 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' @@ -123,6 +124,7 @@ DATABASES = { ###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'] @@ -139,10 +141,11 @@ PASSWORD_HASHERS = ['django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher'] ###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'): @@ -163,7 +166,8 @@ Then, when creating Users, add the permission to that instance of the user model 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. -Problem Code(line 710, 813) +###Repair: +Problem Code: lines 710, 813 ###Solution: Remove the @csrf_exempt decorator from these functions. Ensure that