From af255b66da5b8529a645cd3026777408d69291ac Mon Sep 17 00:00:00 2001 From: Swapnamol Abraham Date: Fri, 28 Mar 2025 12:34:48 +0000 Subject: [PATCH 1/4] File upload vulnerability fixes --- .../Controllers/Api/ResourceController.cs | 27 ++++++++++++ .../Interfaces/IResourceService.cs | 7 +++ .../Scripts/vuesrc/helpers/fileUpload.ts | 2 +- .../Services/ContributeService.cs | 29 +++++++++++- .../Services/ResourceService.cs | 32 ++++++++++++++ .../LearningHub.Nhs.Database.sqlproj | 1 + .../CreateResourceTypeValidationRule.sql | 44 +++++++++++++++++++ .../Post-Deploy/Script.PostDeployment.sql | 1 + 8 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 WebAPI/LearningHub.Nhs.Database/Scripts/LH Database Scripts/CreateResourceTypeValidationRule.sql diff --git a/LearningHub.Nhs.WebUI/Controllers/Api/ResourceController.cs b/LearningHub.Nhs.WebUI/Controllers/Api/ResourceController.cs index 8544241bd..294785798 100644 --- a/LearningHub.Nhs.WebUI/Controllers/Api/ResourceController.cs +++ b/LearningHub.Nhs.WebUI/Controllers/Api/ResourceController.cs @@ -258,6 +258,33 @@ public async Task RecordExternalReferenceUserAgreementAsync([FromB } } + /// + /// The RecordExternalReferenceUserAgreementAsync. + /// + /// model. + /// A representing the result of the asynchronous operation. + [HttpPost] + [Route(" CreateResourceVersionValidationResult")] + public async Task CreateResourceVersionValidationResultAsync([FromBody] ResourceVersionValidationResultViewModel model) + { + await this.resourceService.CreateResourceVersionValidationResultAsync(model); + return this.Ok(); + } + + /// + /// The RecordExternalReferenceUserAgreementAsync. + /// + /// model. + /// A representing the result of the asynchronous operation. + [HttpPost] + [Route(" CreateResourceVersionValidationResults")] + public async Task CreateResourceVersionValidationResultAsync([FromBody] string filename) + { + ResourceVersionValidationResultViewModel model = new ResourceVersionValidationResultViewModel(); + await this.resourceService.CreateResourceVersionValidationResultAsync(model); + return this.Ok(); + } + /// /// The GetHeaderById. /// diff --git a/LearningHub.Nhs.WebUI/Interfaces/IResourceService.cs b/LearningHub.Nhs.WebUI/Interfaces/IResourceService.cs index c09ab0704..f5057402d 100644 --- a/LearningHub.Nhs.WebUI/Interfaces/IResourceService.cs +++ b/LearningHub.Nhs.WebUI/Interfaces/IResourceService.cs @@ -269,6 +269,13 @@ public interface IResourceService /// The . Task CreateResourceVersionProviderAsync(ResourceVersionProviderViewModel model); + /// + /// Creates resource version validation results corresponding to the value in the corresponding input view model. + /// + /// Details of the validation results. + /// A representing the result of the asynchronous operation. + Task CreateResourceVersionValidationResultAsync(ResourceVersionValidationResultViewModel validationResultViewModel); + /// /// Delete resource version provider. /// diff --git a/LearningHub.Nhs.WebUI/Scripts/vuesrc/helpers/fileUpload.ts b/LearningHub.Nhs.WebUI/Scripts/vuesrc/helpers/fileUpload.ts index 907438e2b..a748da737 100644 --- a/LearningHub.Nhs.WebUI/Scripts/vuesrc/helpers/fileUpload.ts +++ b/LearningHub.Nhs.WebUI/Scripts/vuesrc/helpers/fileUpload.ts @@ -49,7 +49,7 @@ const MAX_FILE_SIZE = 10 * 1000 * 1000 * 1000; // 10GB // This list should correspond to the disallowed extensions contained in the FileType table const BLOCKED_FILE_EXTENSIONS = ['.app', '.asp', '.aspx', '.dll', '.dmg', '.exe', '.flv', '.f4v', '.js', '.jsp', - '.php', '.shtm', '.shtml', '.swf','.webm']; + '.php', '.shtm', '.shtml', '.swf', '.webm', '.bat', '.cmd', '.vbs', '.msi', '.pif', '.sh', '.tar', '.gz', '.7z', '.rar', '.sys', '.bak', '.iso', '.torrent']; const IMAGE_FILE_EXTENSIONS = ['.apng', '.avif', '.bmp', '.cur', '.gif', '.ico', '.jfif', '.jpeg', '.jpg', '.pjp', '.pjpeg', '.png', '.psd', '.svg', '.tif', '.tiff', '.webp']; diff --git a/LearningHub.Nhs.WebUI/Services/ContributeService.cs b/LearningHub.Nhs.WebUI/Services/ContributeService.cs index 7da5a8844..a61487e4e 100644 --- a/LearningHub.Nhs.WebUI/Services/ContributeService.cs +++ b/LearningHub.Nhs.WebUI/Services/ContributeService.cs @@ -17,6 +17,8 @@ using LearningHub.Nhs.WebUI.Models.Contribute; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; + using Microsoft.VisualBasic; + using MK.IO.Models; using Newtonsoft.Json; /// @@ -38,7 +40,7 @@ public class ContributeService : BaseService, IContributeServ /// MKIO media service. /// Learning hub http client. /// Logger. - public ContributeService(IFileService fileService, IResourceService resourceService, IAzureMediaService azureMediaService, ILearningHubHttpClient learningHubHttpClient, ILogger logger, IAzureMediaService mediaService) + public ContributeService(IFileService fileService, IResourceService resourceService, IAzureMediaService azureMediaService, ILearningHubHttpClient learningHubHttpClient, ILogger logger, IAzureMediaService mediaService) : base(learningHubHttpClient, logger) { this.fileService = fileService; @@ -502,6 +504,31 @@ public async Task ProcessResourceFileAsync(int resourceVersion if ((fileType == null) || (fileType != null && fileType.NotAllowed) || file.Length <= 0) { + // Define dangerous file extensions + string[] dangerousExtensions = { ".exe", ".dll", ".bat", ".js", ".vbs", ".sh", ".ps1" }; + if (dangerousExtensions.Any(ext => file.FileName.EndsWith(ext, StringComparison.OrdinalIgnoreCase))) + { + var error = $"A potentially harmful file has been detected and blocked: {file.FileName}."; + var validationDetail = new ResourceVersionValidationResultViewModel + { + ResourceVersionId = resourceVersionId, + Success = false, + Details = string.Empty, + AmendUserId = currentUserId, + ResourceVersionValidationRuleResultViewModels = new[] + { + new ResourceVersionValidationRuleResultViewModel + { + ResourceTypeValidationRuleEnum = ResourceTypeValidationRuleEnum.HtmlResource_RootIndexPresent, + Success = false, + Details = error, + }, + }.ToList(), + }; + + await this.resourceService.CreateResourceVersionValidationResultAsync(validationDetail); + } + return new FileUploadResult() { FileName = file.FileName, diff --git a/LearningHub.Nhs.WebUI/Services/ResourceService.cs b/LearningHub.Nhs.WebUI/Services/ResourceService.cs index dfc5d4d17..02a233ea0 100644 --- a/LearningHub.Nhs.WebUI/Services/ResourceService.cs +++ b/LearningHub.Nhs.WebUI/Services/ResourceService.cs @@ -1195,6 +1195,38 @@ public async Task CreateResourceVersionProviderAsyn return apiResponse.ValidationResult; } + /// + /// Creates resource version validation results corresponding to the value in the corresponding input view model. + /// + /// Details of the validation results. + /// A representing the result of the asynchronous operation. + public async Task CreateResourceVersionValidationResultAsync(ResourceVersionValidationResultViewModel validationResultViewModel) + { + ApiResponse apiResponse = null; + var json = JsonConvert.SerializeObject(validationResultViewModel); + var stringContent = new StringContent(json, UnicodeEncoding.UTF8, "application/json"); + + var client = await this.LearningHubHttpClient.GetClientAsync(); + + var request = $"Resource/CreateResourceVersionValidationResult"; + var response = await client.PostAsync(request, stringContent).ConfigureAwait(false); + + if (response.IsSuccessStatusCode) + { + var result = response.Content.ReadAsStringAsync().Result; + apiResponse = JsonConvert.DeserializeObject(result); + + if (!apiResponse.Success) + { + throw new Exception("save failed!"); + } + } + else if (response.StatusCode == System.Net.HttpStatusCode.Unauthorized || response.StatusCode == System.Net.HttpStatusCode.Forbidden) + { + throw new Exception("AccessDenied"); + } + } + /// /// The delete resource version provider. /// diff --git a/WebAPI/LearningHub.Nhs.Database/LearningHub.Nhs.Database.sqlproj b/WebAPI/LearningHub.Nhs.Database/LearningHub.Nhs.Database.sqlproj index 42a432ae2..f957442e6 100644 --- a/WebAPI/LearningHub.Nhs.Database/LearningHub.Nhs.Database.sqlproj +++ b/WebAPI/LearningHub.Nhs.Database/LearningHub.Nhs.Database.sqlproj @@ -536,6 +536,7 @@ + diff --git a/WebAPI/LearningHub.Nhs.Database/Scripts/LH Database Scripts/CreateResourceTypeValidationRule.sql b/WebAPI/LearningHub.Nhs.Database/Scripts/LH Database Scripts/CreateResourceTypeValidationRule.sql new file mode 100644 index 000000000..c86b14264 --- /dev/null +++ b/WebAPI/LearningHub.Nhs.Database/Scripts/LH Database Scripts/CreateResourceTypeValidationRule.sql @@ -0,0 +1,44 @@ +IF NOT EXISTS(SELECT 'X' FROM [resources].[ResourceTypeValidationRule] WHERE Id = 7) +BEGIN + INSERT INTO [resources].[ResourceTypeValidationRule] + ([Id] + ,[ResourceTypeId] + ,[Description] + ,[Deleted] + ,[CreateUserId] + ,[CreateDate] + ,[AmendUserId] + ,[AmendDate]) + VALUES + (7 + ,9 + ,'GenericFile_ValidZipFile' + ,0 + ,4 + ,SYSDATETIMEOFFSET() + ,4 + ,SYSDATETIMEOFFSET()) +END +GO +IF NOT EXISTS(SELECT 'X' FROM [resources].[ResourceTypeValidationRule] WHERE Id = 8) +BEGIN + INSERT INTO [resources].[ResourceTypeValidationRule] + ([Id] + ,[ResourceTypeId] + ,[Description] + ,[Deleted] + ,[CreateUserId] + ,[CreateDate] + ,[AmendUserId] + ,[AmendDate]) + VALUES + (8 + ,9 + ,'GenericFile_ValidFileTypes' + ,0 + ,4 + ,SYSDATETIMEOFFSET() + ,4 + ,SYSDATETIMEOFFSET()) +END +GO diff --git a/WebAPI/LearningHub.Nhs.Database/Scripts/Post-Deploy/Script.PostDeployment.sql b/WebAPI/LearningHub.Nhs.Database/Scripts/Post-Deploy/Script.PostDeployment.sql index 34ba8ad39..0e340dd70 100644 --- a/WebAPI/LearningHub.Nhs.Database/Scripts/Post-Deploy/Script.PostDeployment.sql +++ b/WebAPI/LearningHub.Nhs.Database/Scripts/Post-Deploy/Script.PostDeployment.sql @@ -86,4 +86,5 @@ UPDATE [resources].[ResourceVersion] SET CertificateEnabled = 0 WHERE VersionSta :r .\Scripts\AttributeData.sql :r .\Scripts\PPSXFileType.sql :r .\Scripts\UpdateFileTypes.sql +:r .\Scripts\CreateResourceTypeValidationRule.sql From e4dec15239fac2aab88062ef1ae10d1e11d2fba0 Mon Sep 17 00:00:00 2001 From: Swapnamol Abraham Date: Thu, 10 Apr 2025 14:42:51 +0100 Subject: [PATCH 2/4] TD-3727: Prompting a password entry screen on file upload --- .../Controllers/Api/UserController.cs | 21 +++++ .../Scripts/vuesrc/contribute/Content.vue | 89 +++++++++++++++++-- .../Scripts/vuesrc/data/user.ts | 16 +++- 3 files changed, 120 insertions(+), 6 deletions(-) diff --git a/LearningHub.Nhs.WebUI/Controllers/Api/UserController.cs b/LearningHub.Nhs.WebUI/Controllers/Api/UserController.cs index fc5c5579a..75f260cdb 100644 --- a/LearningHub.Nhs.WebUI/Controllers/Api/UserController.cs +++ b/LearningHub.Nhs.WebUI/Controllers/Api/UserController.cs @@ -98,6 +98,27 @@ public async Task CheckUserRole() return this.Ok(isSystemAdmin); } + /// + /// to check user password is correct. + /// + /// The currentPassword. + /// The . + [HttpGet] + [Route("ConfirmPassword/{currentPassword}")] + public async Task ConfirmPassword(string currentPassword) + { + string passwordHash = this.userService.Base64MD5HashDigest(currentPassword); + var userPersonalDetails = await this.userService.GetCurrentUserPersonalDetailsAsync(); + if (userPersonalDetails != null && userPersonalDetails.PasswordHash == passwordHash) + { + return this.Ok(true); + } + else + { + return this.Ok(false); + } + } + /// /// The GetCurrentUserPersonalDetails. /// diff --git a/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue b/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue index e5a5e2c43..5c4f23c0c 100644 --- a/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue +++ b/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue @@ -376,6 +376,50 @@ +
+ +
+
+
+
+
+

Confirm the password

+
+
+
+

+ To continue with the upload, please verify your identity by entering your password. +

+
+
+
+ +
+ Enter a valid password. +
+ +
+ +
+
+
+
+
+
+ + +
+
+
+
+
+ +
+
@@ -427,6 +471,7 @@ import GenericFileUploader from './GenericFileUploader.vue'; import { UploadResourceType, ResourceType, VersionStatus } from '../constants'; import { resourceData } from '../data/resource'; + import { userData } from '../data/user'; import { FileTypeModel } from "../models/contribute/fileTypeModel"; import { FileUploadResult } from "../models/contribute/FileUploadResult"; import { FlagModel } from '../models/flagModel'; @@ -490,6 +535,8 @@ displayType: '' as string, commonContentKey: 0, avUnavailableMessage: false, + passwordVerification: false, + currentPassword: '', // Some of the Content components have local state // which isn't in the vuex store. // This means those fields are validated using an @@ -573,7 +620,7 @@ closeAfterSave(): boolean { return this.$store.state.closeAfterSave; }, - isFileAlreadyUploaded(): boolean { + isFileAlreadyUploaded(): boolean { switch (this.selectedResourceType) { case this.resourceType.GENERICFILE: return this.$store.state.genericFileDetail.file.fileName !== ''; @@ -739,6 +786,12 @@ this.processPublish(); } }, + submitPassword() { + this.$v.currentPassword.$touch(); + if (!this.$v.currentPassword.$invalid) { + this.validatePassword(); + } + }, async processPublish() { this.showError = false; let publishSuccess = await resourceData.publishResource(this.resourceVersionId, this.publishNotes); @@ -758,6 +811,18 @@ this.errorMessage = "An error occurred whilst trying to publish the resource."; } }, + async validatePassword() { + this.showError = false; + let isValidUser = await userData.IsValidUser(this.currentPassword); + if (isValidUser) { + this.acceptUploadedFile(); + this.passwordVerification = false; + } + else { + this.showError = true; + this.errorMessage = "Enter a valid password."; + } + }, deleteResource() { this.deleteWarning = true; this.showError = false; @@ -822,6 +887,10 @@ this.fileUploadRef.value = null; (this.$refs.fileUploader as any).uploadResourceFile(this.file); }, + confirmPassword() { + this.currentPassword = ''; + this.passwordVerification = true; + }, async fileUploadComplete(uploadResult: FileUploadResult) { if (!uploadResult.invalid) { if (uploadResult.resourceType != ResourceType.SCORM) { @@ -900,9 +969,11 @@ this.fileErrorType = FileErrorTypeEnum.InvalidScormType; return; } - this.acceptUploadedFile(); + this.confirmPassword(); + //this.acceptUploadedFile(); } else { - this.acceptUploadedFile(); + this.confirmPassword(); + //this.acceptUploadedFile(); } } } @@ -948,10 +1019,12 @@ this.fileTypeChangeWarning = true; } } else { - this.acceptUploadedFile(); + this.confirmPassword(); + //this.acceptUploadedFile(); } } else { - this.acceptUploadedFile(); + this.confirmPassword(); + //this.acceptUploadedFile(); } } } @@ -1062,6 +1135,9 @@ }, publishNotes: { required + }, + currentPassword: { + required } }, watch: { @@ -1105,4 +1181,7 @@ max-height: 90vh; overflow-y: auto; } + .password-div-width{ + max-width:70% !important; + } diff --git a/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts b/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts index 472ca9bac..464594253 100644 --- a/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts +++ b/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts @@ -66,6 +66,19 @@ const IsSystemAdmin = async function (): Promise { }); }; +const IsValidUser = async function (currentPassword: string): Promise { + debugger; + var IsValidUser = `/api/User/ConfirmPassword/${currentPassword}`; + return await AxiosWrapper.axios.get(IsValidUser) + .then(response => { + return response.data; + }) + .catch(e => { + console.log('IsValidUser:' + e); + throw e; + }); +}; + const getCurrentUserBasicDetails = async function (): Promise { return await AxiosWrapper.axios.get('/api/User/GetCurrentUserBasicDetails') .then(response => { @@ -173,5 +186,6 @@ export const userData = { keepUserSessionAlive, getkeepUserSessionAliveInterval, isGeneralUser, - IsSystemAdmin + IsSystemAdmin, + IsValidUser } From 583162ac2c1861c8b21e69d67e26aead6da4eded Mon Sep 17 00:00:00 2001 From: Swapnamol Abraham Date: Thu, 10 Apr 2025 17:15:16 +0100 Subject: [PATCH 3/4] Modified the content --- .../Scripts/vuesrc/contribute/Content.vue | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue b/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue index 5c4f23c0c..488594fec 100644 --- a/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue +++ b/LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue @@ -383,12 +383,12 @@
-

Confirm the password

+

Confirm your password

- To continue with the upload, please verify your identity by entering your password. + To continue with the upload please verify your identity by entering your password.

@@ -970,10 +970,8 @@ return; } this.confirmPassword(); - //this.acceptUploadedFile(); } else { this.confirmPassword(); - //this.acceptUploadedFile(); } } } @@ -1020,11 +1018,9 @@ } } else { this.confirmPassword(); - //this.acceptUploadedFile(); } } else { - this.confirmPassword(); - //this.acceptUploadedFile(); + this.confirmPassword(); } } } From 9155364535ffb5cf99817588268462f66de18fdd Mon Sep 17 00:00:00 2001 From: Swapnamol Abraham Date: Fri, 11 Apr 2025 09:03:43 +0100 Subject: [PATCH 4/4] Removed the debugger --- LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts b/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts index 464594253..9f50357fc 100644 --- a/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts +++ b/LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts @@ -67,7 +67,6 @@ const IsSystemAdmin = async function (): Promise { }; const IsValidUser = async function (currentPassword: string): Promise { - debugger; var IsValidUser = `/api/User/ConfirmPassword/${currentPassword}`; return await AxiosWrapper.axios.get(IsValidUser) .then(response => {