From 8c83a18f931239caaa0a0e9a080c8868cde6ba35 Mon Sep 17 00:00:00 2001 From: sbwalker Date: Thu, 6 Feb 2025 08:19:57 -0500 Subject: [PATCH] improve file upload validation and error handling on server --- Oqtane.Server/Controllers/FileController.cs | 123 +++++++++++--------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/Oqtane.Server/Controllers/FileController.cs b/Oqtane.Server/Controllers/FileController.cs index c754b689..4bdb8a90 100644 --- a/Oqtane.Server/Controllers/FileController.cs +++ b/Oqtane.Server/Controllers/FileController.cs @@ -22,6 +22,7 @@ using Microsoft.AspNetCore.Cors; using System.IO.Compression; using Oqtane.Services; using Microsoft.Extensions.Primitives; +using Microsoft.AspNetCore.Http.HttpResults; // ReSharper disable StringIndexOfIsCultureSpecific.1 @@ -430,80 +431,96 @@ namespace Oqtane.Controllers [HttpPost("upload")] public async Task UploadFile([FromForm] string folder, IFormFile formfile) { + if (string.IsNullOrEmpty(folder)) + { + _logger.Log(LogLevel.Error, this, LogFunction.Security, "File Upload Does Not Contain A Folder"); + return StatusCode((int)HttpStatusCode.Forbidden); + } + if (formfile == null || formfile.Length <= 0) { - return NoContent(); + _logger.Log(LogLevel.Error, this, LogFunction.Security, "File Upload Does Not Contain A File"); + return StatusCode((int)HttpStatusCode.Forbidden); } // ensure filename is valid if (!formfile.FileName.IsPathOrFileValid() || !HasValidFileExtension(formfile.FileName)) { - _logger.Log(LogLevel.Error, this, LogFunction.Security, "File Name Is Invalid Or Contains Invalid Extension {File}", formfile.FileName); - return NoContent(); + _logger.Log(LogLevel.Error, this, LogFunction.Security, "File Upload File Name Is Invalid Or Contains Invalid Extension {File}", formfile.FileName); + return StatusCode((int)HttpStatusCode.Forbidden); } // ensure headers exist - if (!Request.Headers.TryGetValue("PartCount", out StringValues partCount) || !Request.Headers.TryGetValue("TotalParts", out StringValues totalParts)) + if (!Request.Headers.TryGetValue("PartCount", out StringValues partcount) || !int.TryParse(partcount, out int partCount) || partCount <= 0 || + !Request.Headers.TryGetValue("TotalParts", out StringValues totalparts) || !int.TryParse(totalparts, out int totalParts) || totalParts <= 0) { - _logger.Log(LogLevel.Error, this, LogFunction.Security, "File Upload Request Is Missing Required Headers"); - return NoContent(); + _logger.Log(LogLevel.Error, this, LogFunction.Security, "File Upload Is Missing Required Headers"); + return StatusCode((int)HttpStatusCode.Forbidden); } - string fileName = formfile.FileName + ".part_" + int.Parse(partCount).ToString("000") + "_" + int.Parse(totalParts).ToString("000"); + // create file name using header values + string fileName = formfile.FileName + ".part_" + partCount.ToString("000") + "_" + totalParts.ToString("000"); string folderPath = ""; - int FolderId; - if (int.TryParse(folder, out FolderId)) + try { - Folder Folder = _folders.GetFolder(FolderId); - if (Folder != null && Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.Edit, Folder.PermissionList)) + int FolderId; + if (int.TryParse(folder, out FolderId)) { - folderPath = _folders.GetFolderPath(Folder); - } - } - else - { - FolderId = -1; - if (User.IsInRole(RoleNames.Host)) - { - folderPath = GetFolderPath(folder); - } - } - - if (!string.IsNullOrEmpty(folderPath)) - { - CreateDirectory(folderPath); - using (var stream = new FileStream(Path.Combine(folderPath, fileName), FileMode.Create)) - { - await formfile.CopyToAsync(stream); - } - - string upload = await MergeFile(folderPath, fileName); - if (upload != "" && FolderId != -1) - { - var file = CreateFile(upload, FolderId, Path.Combine(folderPath, upload)); - if (file != null) + Folder Folder = _folders.GetFolder(FolderId); + if (Folder != null && Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.Edit, Folder.PermissionList)) { - if (file.FileId == 0) - { - file = _files.AddFile(file); - } - else - { - file = _files.UpdateFile(file); - } - _logger.Log(LogLevel.Information, this, LogFunction.Create, "File Uploaded {File}", Path.Combine(folderPath, upload)); - _syncManager.AddSyncEvent(_alias, EntityNames.File, file.FileId, SyncEventActions.Create); + folderPath = _folders.GetFolderPath(Folder); + } + } + else + { + FolderId = -1; + if (User.IsInRole(RoleNames.Host)) + { + folderPath = GetFolderPath(folder); } } - } - else - { - _logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Upload Attempt {Folder} {File}", folder, formfile.FileName); - HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden; - } - return NoContent(); + if (!string.IsNullOrEmpty(folderPath)) + { + CreateDirectory(folderPath); + using (var stream = new FileStream(Path.Combine(folderPath, fileName), FileMode.Create)) + { + await formfile.CopyToAsync(stream); + } + + string upload = await MergeFile(folderPath, fileName); + if (upload != "" && FolderId != -1) + { + var file = CreateFile(upload, FolderId, Path.Combine(folderPath, upload)); + if (file != null) + { + if (file.FileId == 0) + { + file = _files.AddFile(file); + } + else + { + file = _files.UpdateFile(file); + } + _logger.Log(LogLevel.Information, this, LogFunction.Create, "File Uploaded {File}", Path.Combine(folderPath, upload)); + _syncManager.AddSyncEvent(_alias, EntityNames.File, file.FileId, SyncEventActions.Create); + } + } + return NoContent(); + } + else + { + _logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Upload Attempt {Folder} {File}", folder, formfile.FileName); + return StatusCode((int)HttpStatusCode.Forbidden); + } + } + catch (Exception ex) + { + _logger.Log(LogLevel.Error, this, LogFunction.Create, ex, "File Upload Attempt Failed {Folder} {File}", folder, formfile.FileName); + return StatusCode((int)HttpStatusCode.InternalServerError); + } } private async Task MergeFile(string folder, string filename)