From dd74ff5c67e6199f865da6894d7b4de8c8ede19c Mon Sep 17 00:00:00 2001 From: master3395 Date: Wed, 7 Jan 2026 23:46:11 +0100 Subject: [PATCH] Fix issue #1643: Fix downloadFile function to properly parse query parameters - Changed from incorrect URI splitting to proper request.GET.get() method - Added proper URL decoding with unquote() - Fixed both downloadFile and RootDownloadFile functions - Preserved existing security checks (symlink detection, path traversal prevention) - Added path normalization for additional security - Improved error messages to match reported error format This fixes the 'Unauthorized access: Not a valid file' error when downloading files from the file manager. --- filemanager/views.py | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/filemanager/views.py b/filemanager/views.py index d7749ab64..14c75c648 100644 --- a/filemanager/views.py +++ b/filemanager/views.py @@ -307,13 +307,19 @@ def downloadFile(request): try: userID = request.session['userID'] admin = Administrator.objects.get(pk=userID) - from urllib.parse import quote - from django.utils.encoding import iri_to_uri + from urllib.parse import unquote - fileToDownload = request.build_absolute_uri().split('fileToDownload')[1][1:] - fileToDownload = iri_to_uri(fileToDownload) + # Properly get fileToDownload from query parameters + fileToDownload = request.GET.get('fileToDownload') + if not fileToDownload: + return HttpResponse("Unauthorized access: Not a valid file.") + + # URL decode the file path + fileToDownload = unquote(fileToDownload) domainName = request.GET.get('domainName') + if not domainName: + return HttpResponse("Unauthorized access: Domain not specified.") currentACL = ACLManager.loadedACL(userID) @@ -324,8 +330,14 @@ def downloadFile(request): homePath = '/home/%s' % (domainName) - if fileToDownload.find('..') > -1 or fileToDownload.find(homePath) == -1: - return HttpResponse("Unauthorized access.") + # Security checks: prevent directory traversal and ensure file is within domain's home path + if '..' in fileToDownload or not fileToDownload.startswith(homePath): + return HttpResponse("Unauthorized access: Not a valid file.") + + # Normalize path to prevent any path traversal attempts + fileToDownload = os.path.normpath(fileToDownload) + if not fileToDownload.startswith(homePath): + return HttpResponse("Unauthorized access: Not a valid file.") # SECURITY: Check for symlink attacks - resolve the real path and verify it stays within homePath try: @@ -356,11 +368,15 @@ def downloadFile(request): def RootDownloadFile(request): try: userID = request.session['userID'] - from urllib.parse import quote - from django.utils.encoding import iri_to_uri + from urllib.parse import unquote - fileToDownload = request.build_absolute_uri().split('fileToDownload')[1][1:] - fileToDownload = iri_to_uri(fileToDownload) + # Properly get fileToDownload from query parameters + fileToDownload = request.GET.get('fileToDownload') + if not fileToDownload: + return HttpResponse("Unauthorized access: Not a valid file.") + + # URL decode the file path + fileToDownload = unquote(fileToDownload) currentACL = ACLManager.loadedACL(userID) @@ -370,9 +386,12 @@ def RootDownloadFile(request): return ACLManager.loadError() # SECURITY: Prevent path traversal attacks - if fileToDownload.find('..') > -1: + if '..' in fileToDownload: return HttpResponse("Unauthorized access: Path traversal detected.") + # Normalize path to prevent any path traversal attempts + fileToDownload = os.path.normpath(fileToDownload) + # SECURITY: Check for symlink attacks - resolve the real path and verify it's safe try: # Get the real path (resolves symlinks)