Skip to content

Commit b722c48

Browse files
swapnamol-abrahamswapnamol-abraham
authored
Merge pull request #1081 from TechnologyEnhancedLearning/Develop/Fixes/TD-3727-Unrestricted-File-Upload-Vulnerability
Develop/fixes/td 3727 unrestricted file upload vulnerability
2 parents 163853a + c3137a1 commit b722c48

File tree

11 files changed

+256
-8
lines changed

11 files changed

+256
-8
lines changed

‎LearningHub.Nhs.WebUI/Controllers/Api/ResourceController.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,33 @@ public async Task<ActionResult> RecordExternalReferenceUserAgreementAsync([FromB
258258
}
259259
}
260260

261+
/// <summary>
262+
/// The RecordExternalReferenceUserAgreementAsync.
263+
/// </summary>
264+
/// <param name="model">model.</param>
265+
/// <returns>A <see cref="Task{TResult}"/> representing the result of the asynchronous operation.</returns>
266+
[HttpPost]
267+
[Route(" CreateResourceVersionValidationResult")]
268+
public async Task<ActionResult> CreateResourceVersionValidationResultAsync([FromBody] ResourceVersionValidationResultViewModel model)
269+
{
270+
await this.resourceService.CreateResourceVersionValidationResultAsync(model);
271+
return this.Ok();
272+
}
273+
274+
/// <summary>
275+
/// The RecordExternalReferenceUserAgreementAsync.
276+
/// </summary>
277+
/// <param name="filename">model.</param>
278+
/// <returns>A <see cref="Task{TResult}"/> representing the result of the asynchronous operation.</returns>
279+
[HttpPost]
280+
[Route(" CreateResourceVersionValidationResults")]
281+
public async Task<ActionResult> CreateResourceVersionValidationResultAsync([FromBody] string filename)
282+
{
283+
ResourceVersionValidationResultViewModel model = new ResourceVersionValidationResultViewModel();
284+
await this.resourceService.CreateResourceVersionValidationResultAsync(model);
285+
return this.Ok();
286+
}
287+
261288
/// <summary>
262289
/// The GetHeaderById.
263290
/// </summary>

‎LearningHub.Nhs.WebUI/Controllers/Api/UserController.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,27 @@ public async Task<ActionResult> CheckUserRole()
119119
return this.Ok(isSystemAdmin);
120120
}
121121

122+
/// <summary>
123+
/// to check user password is correct.
124+
/// </summary>
125+
/// <param name="currentPassword">The currentPassword.</param>
126+
/// <returns>The <see cref="Task{ActionResult}"/>.</returns>
127+
[HttpGet]
128+
[Route("ConfirmPassword/{currentPassword}")]
129+
public async Task<ActionResult> ConfirmPassword(string currentPassword)
130+
{
131+
string passwordHash = this.userService.Base64MD5HashDigest(currentPassword);
132+
var userPersonalDetails = await this.userService.GetCurrentUserPersonalDetailsAsync();
133+
if (userPersonalDetails != null && userPersonalDetails.PasswordHash == passwordHash)
134+
{
135+
return this.Ok(true);
136+
}
137+
else
138+
{
139+
return this.Ok(false);
140+
}
141+
}
142+
122143
/// <summary>
123144
/// The GetCurrentUserPersonalDetails.
124145
/// </summary>

‎LearningHub.Nhs.WebUI/Interfaces/IResourceService.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,13 @@ public interface IResourceService
269269
/// <returns>The <see cref="Task{LearningHubValidationResult}"/>.</returns>
270270
Task<LearningHubValidationResult> CreateResourceVersionProviderAsync(ResourceVersionProviderViewModel model);
271271

272+
/// <summary>
273+
/// Creates resource version validation results corresponding to the value in the corresponding input view model.
274+
/// </summary>
275+
/// <param name="validationResultViewModel">Details of the validation results.</param>
276+
/// <returns>A <see cref="Task"/> representing the result of the asynchronous operation.</returns>
277+
Task CreateResourceVersionValidationResultAsync(ResourceVersionValidationResultViewModel validationResultViewModel);
278+
272279
/// <summary>
273280
/// Delete resource version provider.
274281
/// </summary>

‎LearningHub.Nhs.WebUI/Scripts/vuesrc/contribute/Content.vue‎

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,50 @@
376376
</transition>
377377
</div>
378378

379+
<div v-if="passwordVerification">
380+
<transition name="modal">
381+
<div class="modal-mask">
382+
<div class="modal-wrapper">
383+
<div class="modal-dialog">
384+
<div class="modal-content">
385+
<div class="modal-header text-center">
386+
<h4 class="modal-title"><i class="warningTriangle fa-solid fa-triangle-exclamation"></i> Confirm your password</h4>
387+
</div>
388+
<div class="modal-body">
389+
<div class="model-body-container">
390+
<p>
391+
To continue with the upload please verify your identity by entering your password.
392+
</p>
393+
</div>
394+
<div>
395+
<div class="form-group password-div-width" v-bind:class="{ 'input-validation-error': $v.currentPassword.$error}">
396+
<label for="confirmPassword" class="pt-10">Current password</label>
397+
<div class="error-text" v-if="$v.currentPassword.$invalid && $v.currentPassword.$dirty">
398+
<span class="text-danger">Enter a valid password.</span>
399+
</div>
400+
<input type="password" class="form-control" id="currentPassword" aria-describedby="currentPassword" autocomplete="off" maxlength="1000"
401+
v-model.trim="currentPassword"
402+
placeholder="Current password"
403+
@blur="$v.currentPassword.$touch()"
404+
v-bind:class="{ 'input-validation-error': $v.currentPassword.$error}">
405+
</div>
406+
407+
<div class="row" v-if="showError">
408+
<div class="form-group col-12 text-danger" v-html="errorMessage" />
409+
</div>
410+
</div>
411+
</div>
412+
<div class="modal-footer modal-footer--buttons">
413+
<button type="button" class="nhsuk-button nhsuk-button--secondary" @click="passwordVerification=false">Cancel</button>
414+
<button type="button" class="nhsuk-button" @click="submitPassword">Continue</button>
415+
</div>
416+
</div>
417+
</div>
418+
</div>
419+
</div>
420+
</transition>
421+
</div>
422+
379423
</div>
380424

381425
<div v-if="!resourceLoading" class="limit-width px-xl-0 mx-xl-0 pb-5">
@@ -427,6 +471,7 @@
427471
import GenericFileUploader from './GenericFileUploader.vue';
428472
import { UploadResourceType, ResourceType, VersionStatus } from '../constants';
429473
import { resourceData } from '../data/resource';
474+
import { userData } from '../data/user';
430475
import { FileTypeModel } from "../models/contribute/fileTypeModel";
431476
import { FileUploadResult } from "../models/contribute/FileUploadResult";
432477
import { FlagModel } from '../models/flagModel';
@@ -490,6 +535,8 @@
490535
displayType: '' as string,
491536
commonContentKey: 0,
492537
avUnavailableMessage: false,
538+
passwordVerification: false,
539+
currentPassword: '',
493540
// Some of the Content components have local state
494541
// which isn't in the vuex store.
495542
// This means those fields are validated using an
@@ -573,7 +620,7 @@
573620
closeAfterSave(): boolean {
574621
return this.$store.state.closeAfterSave;
575622
},
576-
isFileAlreadyUploaded(): boolean {
623+
isFileAlreadyUploaded(): boolean {
577624
switch (this.selectedResourceType) {
578625
case this.resourceType.GENERICFILE:
579626
return this.$store.state.genericFileDetail.file.fileName !== '';
@@ -739,6 +786,12 @@
739786
this.processPublish();
740787
}
741788
},
789+
submitPassword() {
790+
this.$v.currentPassword.$touch();
791+
if (!this.$v.currentPassword.$invalid) {
792+
this.validatePassword();
793+
}
794+
},
742795
async processPublish() {
743796
this.showError = false;
744797
let publishSuccess = await resourceData.publishResource(this.resourceVersionId, this.publishNotes);
@@ -758,6 +811,18 @@
758811
this.errorMessage = "An error occurred whilst trying to publish the resource.";
759812
}
760813
},
814+
async validatePassword() {
815+
this.showError = false;
816+
let isValidUser = await userData.IsValidUser(this.currentPassword);
817+
if (isValidUser) {
818+
this.acceptUploadedFile();
819+
this.passwordVerification = false;
820+
}
821+
else {
822+
this.showError = true;
823+
this.errorMessage = "Enter a valid password.";
824+
}
825+
},
761826
deleteResource() {
762827
this.deleteWarning = true;
763828
this.showError = false;
@@ -822,6 +887,10 @@
822887
this.fileUploadRef.value = null;
823888
(this.$refs.fileUploader as any).uploadResourceFile(this.file);
824889
},
890+
confirmPassword() {
891+
this.currentPassword = '';
892+
this.passwordVerification = true;
893+
},
825894
async fileUploadComplete(uploadResult: FileUploadResult) {
826895
if (!uploadResult.invalid) {
827896
if (uploadResult.resourceType != ResourceType.SCORM) {
@@ -900,9 +969,9 @@
900969
this.fileErrorType = FileErrorTypeEnum.InvalidScormType;
901970
return;
902971
}
903-
this.acceptUploadedFile();
972+
this.confirmPassword();
904973
} else {
905-
this.acceptUploadedFile();
974+
this.confirmPassword();
906975
}
907976
}
908977
}
@@ -948,10 +1017,10 @@
9481017
this.fileTypeChangeWarning = true;
9491018
}
9501019
} else {
951-
this.acceptUploadedFile();
1020+
this.confirmPassword();
9521021
}
9531022
} else {
954-
this.acceptUploadedFile();
1023+
this.confirmPassword();
9551024
}
9561025
}
9571026
}
@@ -1062,6 +1131,9 @@
10621131
},
10631132
publishNotes: {
10641133
required
1134+
},
1135+
currentPassword: {
1136+
required
10651137
}
10661138
},
10671139
watch: {
@@ -1105,4 +1177,7 @@
11051177
max-height: 90vh;
11061178
overflow-y: auto;
11071179
}
1180+
.password-div-width{
1181+
max-width:70% !important;
1182+
}
11081183
</style>

‎LearningHub.Nhs.WebUI/Scripts/vuesrc/data/user.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ const IsSystemAdmin = async function (): Promise<boolean[]> {
6666
});
6767
};
6868

69+
const IsValidUser = async function (currentPassword: string): Promise<boolean[]> {
70+
var IsValidUser = `/api/User/ConfirmPassword/${currentPassword}`;
71+
return await AxiosWrapper.axios.get<boolean[]>(IsValidUser)
72+
.then(response => {
73+
return response.data;
74+
})
75+
.catch(e => {
76+
console.log('IsValidUser:' + e);
77+
throw e;
78+
});
79+
};
80+
6981
const getCurrentUserBasicDetails = async function (): Promise<UserBasicModel> {
7082
return await AxiosWrapper.axios.get<UserBasicModel>('/api/User/GetCurrentUserBasicDetails')
7183
.then(response => {
@@ -173,5 +185,6 @@ export const userData = {
173185
keepUserSessionAlive,
174186
getkeepUserSessionAliveInterval,
175187
isGeneralUser,
176-
IsSystemAdmin
188+
IsSystemAdmin,
189+
IsValidUser
177190
}

‎LearningHub.Nhs.WebUI/Scripts/vuesrc/helpers/fileUpload.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ const MAX_FILE_SIZE = 10 * 1000 * 1000 * 1000; // 10GB
4949

5050
// This list should correspond to the disallowed extensions contained in the FileType table
5151
const BLOCKED_FILE_EXTENSIONS = ['.app', '.asp', '.aspx', '.dll', '.dmg', '.exe', '.flv', '.f4v', '.js', '.jsp',
52-
'.php', '.shtm', '.shtml', '.swf','.webm'];
52+
'.php', '.shtm', '.shtml', '.swf', '.webm', '.bat', '.cmd', '.vbs', '.msi', '.pif', '.sh', '.tar', '.gz', '.7z', '.rar', '.sys', '.bak', '.iso', '.torrent'];
5353

5454
const IMAGE_FILE_EXTENSIONS = ['.apng', '.avif', '.bmp', '.cur', '.gif', '.ico', '.jfif', '.jpeg', '.jpg', '.pjp',
5555
'.pjpeg', '.png', '.psd', '.svg', '.tif', '.tiff', '.webp'];

‎LearningHub.Nhs.WebUI/Services/ContributeService.cs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
using LearningHub.Nhs.WebUI.Models.Contribute;
1818
using Microsoft.AspNetCore.Http;
1919
using Microsoft.Extensions.Logging;
20+
using Microsoft.VisualBasic;
21+
using MK.IO.Models;
2022
using Newtonsoft.Json;
2123

2224
/// <summary>
@@ -38,7 +40,7 @@ public class ContributeService : BaseService<ContributeService>, IContributeServ
3840
/// <param name="mediaService">MKIO media service.</param>
3941
/// <param name="learningHubHttpClient">Learning hub http client.</param>
4042
/// <param name="logger">Logger.</param>
41-
public ContributeService(IFileService fileService, IResourceService resourceService, IAzureMediaService azureMediaService, ILearningHubHttpClient learningHubHttpClient, ILogger<ContributeService> logger, IAzureMediaService mediaService)
43+
public ContributeService(IFileService fileService, IResourceService resourceService, IAzureMediaService azureMediaService, ILearningHubHttpClient learningHubHttpClient, ILogger<ContributeService> logger, IAzureMediaService mediaService)
4244
: base(learningHubHttpClient, logger)
4345
{
4446
this.fileService = fileService;
@@ -502,6 +504,31 @@ public async Task<FileUploadResult> ProcessResourceFileAsync(int resourceVersion
502504

503505
if ((fileType == null) || (fileType != null && fileType.NotAllowed) || file.Length <= 0)
504506
{
507+
// Define dangerous file extensions
508+
string[] dangerousExtensions = { ".exe", ".dll", ".bat", ".js", ".vbs", ".sh", ".ps1" };
509+
if (dangerousExtensions.Any(ext => file.FileName.EndsWith(ext, StringComparison.OrdinalIgnoreCase)))
510+
{
511+
var error = $"A potentially harmful file has been detected and blocked: {file.FileName}.";
512+
var validationDetail = new ResourceVersionValidationResultViewModel
513+
{
514+
ResourceVersionId = resourceVersionId,
515+
Success = false,
516+
Details = string.Empty,
517+
AmendUserId = currentUserId,
518+
ResourceVersionValidationRuleResultViewModels = new[]
519+
{
520+
new ResourceVersionValidationRuleResultViewModel
521+
{
522+
ResourceTypeValidationRuleEnum = ResourceTypeValidationRuleEnum.HtmlResource_RootIndexPresent,
523+
Success = false,
524+
Details = error,
525+
},
526+
}.ToList(),
527+
};
528+
529+
await this.resourceService.CreateResourceVersionValidationResultAsync(validationDetail);
530+
}
531+
505532
return new FileUploadResult()
506533
{
507534
FileName = file.FileName,

‎LearningHub.Nhs.WebUI/Services/ResourceService.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,38 @@ public async Task<LearningHubValidationResult> CreateResourceVersionProviderAsyn
11951195
return apiResponse.ValidationResult;
11961196
}
11971197

1198+
/// <summary>
1199+
/// Creates resource version validation results corresponding to the value in the corresponding input view model.
1200+
/// </summary>
1201+
/// <param name="validationResultViewModel">Details of the validation results.</param>
1202+
/// <returns>A <see cref="Task"/> representing the result of the asynchronous operation.</returns>
1203+
public async Task CreateResourceVersionValidationResultAsync(ResourceVersionValidationResultViewModel validationResultViewModel)
1204+
{
1205+
ApiResponse apiResponse = null;
1206+
var json = JsonConvert.SerializeObject(validationResultViewModel);
1207+
var stringContent = new StringContent(json, UnicodeEncoding.UTF8, "application/json");
1208+
1209+
var client = await this.LearningHubHttpClient.GetClientAsync();
1210+
1211+
var request = $"Resource/CreateResourceVersionValidationResult";
1212+
var response = await client.PostAsync(request, stringContent).ConfigureAwait(false);
1213+
1214+
if (response.IsSuccessStatusCode)
1215+
{
1216+
var result = response.Content.ReadAsStringAsync().Result;
1217+
apiResponse = JsonConvert.DeserializeObject<ApiResponse>(result);
1218+
1219+
if (!apiResponse.Success)
1220+
{
1221+
throw new Exception("save failed!");
1222+
}
1223+
}
1224+
else if (response.StatusCode == System.Net.HttpStatusCode.Unauthorized || response.StatusCode == System.Net.HttpStatusCode.Forbidden)
1225+
{
1226+
throw new Exception("AccessDenied");
1227+
}
1228+
}
1229+
11981230
/// <summary>
11991231
/// The delete resource version provider.
12001232
/// </summary>

‎WebAPI/LearningHub.Nhs.Database/LearningHub.Nhs.Database.sqlproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@
537537
<None Include="Scripts\Post-Deploy\Scripts\UpdateFileTypes.sql" />
538538
<None Include="Scripts\Post-Deploy\Scripts\AddDigitalLearningSolutionsExternalSystem.sql" />
539539
<Build Include="Stored Procedures\Activity\GetAssessmentActivityCompletionPercentage.sql" />
540+
<None Include="Scripts\LH Database Scripts\CreateResourceTypeValidationRule.sql" />
540541
<Build Include="Tables\Hub\PasswordResetRequests.sql" />
541542
<Build Include="Stored Procedures\External\ExternalSystemUserCreate.sql" />
542543
<None Include="Scripts\ELFH Database Scripts\ConCurrentUserSessionsTablesandSP.sql" />

0 commit comments

Comments
 (0)