Skip to content

Commit

Permalink
fix: file upload security issue
Browse files Browse the repository at this point in the history
  • Loading branch information
hinakhadim committed Dec 5, 2023
1 parent bd7ef76 commit 55468da
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
6 changes: 6 additions & 0 deletions cms/djangoapps/contentstore/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@ class AssetSizeTooLargeException(Exception):
"""
Raised when the size of an uploaded asset exceeds the maximum size limit.
"""


class InvalidFileTypeException(Exception):
"""
Raised when the type of file is not included in mimetypes
"""
26 changes: 23 additions & 3 deletions cms/djangoapps/contentstore/views/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import json
import logging
import math
import mimetypes
import re
from functools import partial
from urllib.parse import urljoin
Expand All @@ -30,7 +31,7 @@
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order

from ..exceptions import AssetNotFoundException, AssetSizeTooLargeException
from ..exceptions import AssetNotFoundException, AssetSizeTooLargeException, InvalidFileTypeException
from ..utils import reverse_course_url

__all__ = ['assets_handler']
Expand All @@ -45,6 +46,10 @@
}


mimetypes.init()
all_mimetypes = list(mimetypes.types_map.values()) + ['text/javascript', 'text/php']


@login_required
@ensure_csrf_cookie
def assets_handler(request, course_key_string=None, asset_key_string=None):
Expand Down Expand Up @@ -445,15 +450,30 @@ def _get_error_if_course_does_not_exist(course_key): # lint-amnesty, pylint: di
return HttpResponseBadRequest()


def _get_sanitized_filename(filename):
splitted_filename = filename.split('.')
extension = splitted_filename[-1]

# for files with multiple extensions
filename = splitted_filename[:len(splitted_filename) - 1]
filename = "".join(filename)
return "".join(x for x in filename if x.isalnum()) + "." + extension


def _validate_mimetype(file_content_type):
if file_content_type in all_mimetypes: return file_content_type
raise InvalidFileTypeException('{} of filetype is not supported'.format(file_content_type))


def _get_file_metadata_as_dictionary(upload_file): # lint-amnesty, pylint: disable=missing-function-docstring
# compute a 'filename' which is similar to the location formatting; we're
# using the 'filename' nomenclature since we're using a FileSystem paradigm
# here; we're just imposing the Location string formatting expectations to
# keep things a bit more consistent
return {
'upload_file': upload_file,
'filename': upload_file.name,
'mime_type': upload_file.content_type,
'filename': _get_sanitized_filename(upload_file.name),
'mime_type': _validate_mimetype(upload_file.content_type),
'upload_file_size': get_file_size(upload_file)
}

Expand Down

0 comments on commit 55468da

Please sign in to comment.