Skip to content

Commit 2332421

Browse files
committed
fix: Address critical security vulnerabilities from issue #121
**Vulnerability 1: Exposed Authentication Data** - Created UserSafeDTO to sanitize API responses - Removed sensitive fields (passwordHash, securityStamp, concurrencyStamp) from all user endpoints - Updated 5 endpoints: GetUserByID, GetUserByName, GetUserRange, GetUserList, Search - Files: AccountController.cs, ViewModels/Account/UserSafeDTO.cs **Vulnerability 2: Missing File Upload Size Restrictions** - Created FileValidationSettings configuration class - Added per-file size limits: 5MB (profile images), 100MB (project files), 50MB (notebooks) - Implemented configuration via appsettings.json - Added validation checks to all upload endpoints before storing files - Files: FileValidationSettings.cs, appsettings.json, ProjectController.cs, AccountController.cs **Vulnerability 3: No File Type Validation** - Created FileTypeValidator with allowlist-based validation - Implemented file signature/magic number verification - Profile images: .jpg, .jpeg, .png, .gif, .webp only - Project files: .csv, .json, .txt, .xlsx, .pdf, .xml, .tsv, .dat only - Notebooks: .ipynb only - Blocked dangerous extensions: .exe, .bat, .cmd, .sh, .ps1, etc. - Applied validation to all 5 upload endpoints - Files: FileTypeValidator.cs, ProjectController.cs, AccountController.cs **Endpoints Fixed:** - ProjectController: UploadFile, UploadNotebook, UploadNotebookNewVersion, UploadExistingNotebook - AccountController: UploadProfileImage, GetUserByID, GetUserByName, GetUserRange, GetUserList, Search **Security Improvements:** - 100% file validation at API boundary before database storage - Existing user quota checks remain as additional security layer - Configuration-driven approach allows easy adjustment of limits - No breaking changes - only response DTOs and validation logic modified Closes #121
1 parent afc5ee6 commit 2332421

7 files changed

Lines changed: 354 additions & 10 deletions

File tree

README.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,24 @@ Analysim requires two databases to operate: one SQL database (PostgreSQL) for re
7878
"AdminUsers": [
7979
"ADMIN",
8080
"XXX"
81-
]
81+
],
82+
"FileValidation": {
83+
"MaxProfileImageSize": 5242880,
84+
"MaxProjectFileSize": 104857600,
85+
"MaxNotebookFileSize": 52428800,
86+
"AllowedImageExtensions": [".jpg", ".jpeg", ".png", ".gif", ".webp"],
87+
"AllowedProjectFileExtensions": [".csv", ".json", ".txt", ".xlsx", ".xls", ".pdf", ".xml", ".tsv", ".dat"],
88+
"AllowedNotebookExtensions": [".ipynb"],
89+
"BlockedExtensions": [".exe", ".bat", ".cmd", ".sh", ".ps1", ".app", ".dll", ".so", ".dmg", ".pkg", ".msi", ".deb", ".rpm", ".apk", ".zip", ".rar", ".7z", ".tar", ".gz", ".scr", ".vbs", ".js", ".py", ".rb", ".pl"]
90+
}
8291
}
8392

8493
```
8594

95+
#### File validation settings
96+
97+
The `FileValidation` section controls the file upload validation rules. You can customize the maximum file sizes (in bytes) and the lists of allowed/blocked file extensions for profile images, project data files, and notebook uploads. These settings are read at startup and injected into the controllers via dependency injection.
98+
8699
#### Adding admin users
87100

88101
Admin access in Analysim is controlled through the AdminUsers section of the `appsettings.json` and `appsettings.Development.json`. Each entry in the list corresponds to the username of a registered Analysim user. Admin users will see an Admin link in the navigation bar and can access the /admin section of the platform. To add or remove admin privileges, simply update this list and restart the server.
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.IO;
4+
using System.Linq;
5+
6+
namespace Core.Helper
7+
{
8+
/// <summary>
9+
/// Validates file uploads based on extension, MIME type, and file signature
10+
/// Uses an allowlist approach to prevent malicious file uploads
11+
/// </summary>
12+
public class FileTypeValidator
13+
{
14+
private readonly FileValidationSettings _settings;
15+
16+
/// <summary>
17+
/// Magic numbers (file signatures) for common file types
18+
/// Used to verify that file content matches the claimed extension
19+
/// </summary>
20+
private static readonly Dictionary<byte[], string> FileMagicNumbers = new Dictionary<byte[], string>
21+
{
22+
{ new byte[] { 0xFF, 0xD8, 0xFF }, ".jpg" }, // JPEG
23+
{ new byte[] { 0x89, 0x50, 0x4E, 0x47 }, ".png" }, // PNG
24+
{ new byte[] { 0x47, 0x49, 0x46 }, ".gif" }, // GIF
25+
{ new byte[] { 0x52, 0x49, 0x46, 0x46 }, ".webp" }, // WEBP
26+
{ new byte[] { 0x50, 0x4B, 0x03, 0x04 }, ".xlsx" }, // XLSX (zip-based)
27+
{ new byte[] { 0x50, 0x4B, 0x03, 0x04 }, ".zip" }, // ZIP
28+
{ new byte[] { 0x25, 0x50, 0x44, 0x46 }, ".pdf" }, // PDF
29+
{ new byte[] { 0x7B, 0x0A }, ".json" }, // JSON (basic check)
30+
{ new byte[] { 0x23, 0x0A }, ".ipynb" } // Jupyter (text-based)
31+
};
32+
33+
public FileTypeValidator(FileValidationSettings settings)
34+
{
35+
_settings = settings ?? throw new ArgumentNullException(nameof(settings));
36+
}
37+
38+
/// <summary>
39+
/// Validates a profile image upload
40+
/// </summary>
41+
public (bool IsValid, string ErrorMessage) ValidateProfileImage(string fileName, byte[] fileContent)
42+
{
43+
return ValidateFile(fileName, fileContent, _settings.AllowedImageExtensions, maxSize: _settings.MaxProfileImageSize);
44+
}
45+
46+
/// <summary>
47+
/// Validates a project file upload
48+
/// </summary>
49+
public (bool IsValid, string ErrorMessage) ValidateProjectFile(string fileName, byte[] fileContent)
50+
{
51+
return ValidateFile(fileName, fileContent, _settings.AllowedProjectFileExtensions, maxSize: _settings.MaxProjectFileSize);
52+
}
53+
54+
/// <summary>
55+
/// Validates a notebook file upload
56+
/// </summary>
57+
public (bool IsValid, string ErrorMessage) ValidateNotebookFile(string fileName, byte[] fileContent)
58+
{
59+
// Notebooks must be .ipynb only
60+
var result = ValidateFile(fileName, fileContent, _settings.AllowedNotebookExtensions, maxSize: _settings.MaxNotebookFileSize);
61+
62+
if (!result.IsValid)
63+
return result;
64+
65+
// Additional validation: Notebooks should be JSON text files
66+
if (!IsValidJsonContent(fileContent))
67+
return (false, "Invalid Jupyter notebook format. Must be a valid JSON file.");
68+
69+
return (true, string.Empty);
70+
}
71+
72+
/// <summary>
73+
/// Generic file validation
74+
/// </summary>
75+
private (bool IsValid, string ErrorMessage) ValidateFile(
76+
string fileName,
77+
byte[] fileContent,
78+
List<string> allowedExtensions,
79+
int maxSize)
80+
{
81+
if (fileContent == null || fileContent.Length == 0)
82+
return (false, "File content is empty.");
83+
84+
// Check file size
85+
if (fileContent.Length > maxSize)
86+
return (false, $"File size exceeds maximum allowed size of {maxSize} bytes.");
87+
88+
// Get extension
89+
var extension = Path.GetExtension(fileName)?.ToLower();
90+
if (string.IsNullOrEmpty(extension))
91+
return (false, "File has no extension.");
92+
93+
// Check if extension is blocked
94+
if (_settings.BlockedExtensions?.Contains(extension) == true)
95+
return (false, $"File type '{extension}' is not allowed.");
96+
97+
// Check if extension is in allowlist
98+
if (!allowedExtensions.Contains(extension))
99+
return (false, $"File type '{extension}' is not allowed. Allowed types: {string.Join(", ", allowedExtensions)}");
100+
101+
// Verify file signature matches extension (magic number check)
102+
if (!VerifyFileSignature(fileContent, extension))
103+
return (false, $"File content does not match the file extension. Possible file type mismatch or corruption.");
104+
105+
return (true, string.Empty);
106+
}
107+
108+
/// <summary>
109+
/// Verifies file signature (magic number) matches the claimed extension
110+
/// </summary>
111+
private bool VerifyFileSignature(byte[] fileContent, string extension)
112+
{
113+
if (fileContent == null || fileContent.Length == 0)
114+
return false;
115+
116+
// For text-based formats, perform basic validation
117+
if (extension == ".json" || extension == ".ipynb")
118+
return IsValidJsonContent(fileContent);
119+
120+
if (extension == ".csv" || extension == ".txt" || extension == ".tsv" || extension == ".dat")
121+
return IsValidTextContent(fileContent);
122+
123+
// For binary formats, check magic numbers
124+
foreach (var kvp in FileMagicNumbers)
125+
{
126+
if (fileContent.Length >= kvp.Key.Length &&
127+
fileContent.Take(kvp.Key.Length).SequenceEqual(kvp.Key))
128+
{
129+
return kvp.Value == extension || (kvp.Value == ".xlsx" && extension == ".xls");
130+
}
131+
}
132+
133+
// If no magic number matched, assume it's okay for formats we can't verify
134+
// (like CSV, TXT, XML without specific magic numbers)
135+
if (extension == ".xml")
136+
return IsValidTextContent(fileContent) && fileContent.ToString().Contains("<");
137+
138+
return true; // Allow if we can't determine a signature
139+
}
140+
141+
/// <summary>
142+
/// Checks if file content is valid JSON
143+
/// </summary>
144+
private bool IsValidJsonContent(byte[] fileContent)
145+
{
146+
try
147+
{
148+
var text = System.Text.Encoding.UTF8.GetString(fileContent);
149+
var trimmed = text.Trim();
150+
151+
// Basic JSON structure check
152+
return (trimmed.StartsWith("{") && trimmed.EndsWith("}")) ||
153+
(trimmed.StartsWith("[") && trimmed.EndsWith("]"));
154+
}
155+
catch
156+
{
157+
return false;
158+
}
159+
}
160+
161+
/// <summary>
162+
/// Checks if file content is valid text
163+
/// </summary>
164+
private bool IsValidTextContent(byte[] fileContent)
165+
{
166+
try
167+
{
168+
// Attempt to decode as UTF-8
169+
var text = System.Text.Encoding.UTF8.GetString(fileContent);
170+
171+
// Check for null characters (indicator of binary content)
172+
return !text.Contains("\0");
173+
}
174+
catch
175+
{
176+
return false;
177+
}
178+
}
179+
}
180+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
using System.Collections.Generic;
2+
3+
namespace Core.Helper
4+
{
5+
/// <summary>
6+
/// Configuration settings for file validation
7+
/// Loaded from appsettings.json FileValidation section
8+
/// </summary>
9+
public class FileValidationSettings
10+
{
11+
/// <summary>
12+
/// Maximum size for profile image uploads in bytes (default: 5 MB)
13+
/// </summary>
14+
public int MaxProfileImageSize { get; set; } = 5 * 1024 * 1024; // 5 MB
15+
16+
/// <summary>
17+
/// Maximum size for project file uploads in bytes (default: 100 MB)
18+
/// </summary>
19+
public int MaxProjectFileSize { get; set; } = 100 * 1024 * 1024; // 100 MB
20+
21+
/// <summary>
22+
/// Maximum size for notebook file uploads in bytes (default: 50 MB)
23+
/// </summary>
24+
public int MaxNotebookFileSize { get; set; } = 50 * 1024 * 1024; // 50 MB
25+
26+
/// <summary>
27+
/// Allowed file extensions for profile images
28+
/// </summary>
29+
public List<string> AllowedImageExtensions { get; set; } = new List<string>
30+
{
31+
".jpg", ".jpeg", ".png", ".gif", ".webp"
32+
};
33+
34+
/// <summary>
35+
/// Allowed file extensions for project data files
36+
/// </summary>
37+
public List<string> AllowedProjectFileExtensions { get; set; } = new List<string>
38+
{
39+
".csv", ".json", ".txt", ".xlsx", ".xls", ".pdf", ".xml", ".tsv", ".dat"
40+
};
41+
42+
/// <summary>
43+
/// Allowed file extensions for notebook files
44+
/// </summary>
45+
public List<string> AllowedNotebookExtensions { get; set; } = new List<string>
46+
{
47+
".ipynb"
48+
};
49+
50+
/// <summary>
51+
/// File extensions to block (dangerous executables and scripts)
52+
/// </summary>
53+
public List<string> BlockedExtensions { get; set; } = new List<string>
54+
{
55+
".exe", ".bat", ".cmd", ".sh", ".ps1", ".app", ".dll", ".so", ".dmg",
56+
".pkg", ".msi", ".deb", ".rpm", ".apk", ".zip", ".rar", ".7z", ".tar",
57+
".gz", ".scr", ".vbs", ".js", ".py", ".rb", ".pl"
58+
};
59+
}
60+
}

src/Analysim.Web/Controllers/AccountController.cs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,21 @@ public class AccountController : ControllerBase
4141
private readonly ApplicationDbContext _dbContext;
4242
private readonly ILoggerManager _loggerManager;
4343
private readonly IMailNetService _mailNetService;
44+
private readonly FileValidationSettings _fileValidationSettings;
4445

4546
private readonly IConfiguration _configuration;
4647

4748
public AccountController(IOptions<JwtSettings> jwtSettings, UserManager<User> userManager,
4849
SignInManager<User> signManager, ApplicationDbContext dbContext,
4950
ILoggerManager loggerManager,
50-
IMailNetService mailNetService,IConfiguration configuration)
51+
IMailNetService mailNetService,IConfiguration configuration,
52+
IOptions<FileValidationSettings> fileValidationSettings)
5153
{
5254
_jwtSettings = jwtSettings.Value;
5355
_userManager = userManager;
5456
_signManager = signManager;
5557
_dbContext = dbContext;
58+
_fileValidationSettings = fileValidationSettings.Value;
5659
_loggerManager = loggerManager;
5760
_mailNetService = mailNetService;
5861
_configuration = configuration;
@@ -79,7 +82,7 @@ public IActionResult GetUserByID([FromRoute] int id)
7982
// user.EmailConfirmed ;
8083
return Ok(new
8184
{
82-
result = user,
85+
result = ViewModels.Account.UserSafeDTO.FromUser(user),
8386
message = "Received User: " + user.UserName
8487
});
8588
}
@@ -126,7 +129,7 @@ public IActionResult GetUserByName([FromRoute] string username)
126129
if (user == null) return NotFound(new { message = "User Not Found" });
127130
return Ok(new
128131
{
129-
result = user,
132+
result = ViewModels.Account.UserSafeDTO.FromUser(user),
130133
message = "Received User: " + user.UserName
131134
});
132135
}
@@ -152,7 +155,7 @@ public IActionResult GetUserRange([FromQuery(Name = "id")] List<int> ids)
152155

153156
return Ok(new
154157
{
155-
result = users,
158+
result = ViewModels.Account.UserSafeDTO.FromUsers(users),
156159
message = "Received User Range"
157160
});
158161
}
@@ -176,7 +179,7 @@ public IActionResult GetUserList()
176179

177180
return Ok(new
178181
{
179-
result = users,
182+
result = ViewModels.Account.UserSafeDTO.FromUsers(users),
180183
message = "Received User List"
181184
});
182185
}
@@ -224,7 +227,7 @@ public IActionResult Search([FromQuery(Name = "term")] List<string> searchTerms)
224227

225228
return Ok(new
226229
{
227-
result = matchedUser,
230+
result = ViewModels.Account.UserSafeDTO.FromUsers(matchedUser.ToList()),
228231
message = "Search Successful"
229232
});
230233
}
@@ -783,6 +786,12 @@ public async Task<IActionResult> UploadProfileImage([FromForm] AccountUploadVM f
783786
await formdata.File.CopyToAsync(memoryStream);
784787
var fileContent = memoryStream.ToArray();
785788

789+
// Validate file type and size for profile image
790+
var fileValidator = new Core.Helper.FileTypeValidator(_fileValidationSettings);
791+
var validationResult = fileValidator.ValidateProfileImage(formdata.File.FileName, fileContent);
792+
if (!validationResult.IsValid)
793+
return BadRequest(validationResult.ErrorMessage);
794+
786795
// Check For Existing
787796
var blobFile = _dbContext.BlobFiles.FirstOrDefault(x => x.UserID == user.Id && x.Name == "profileImage");
788797
if (blobFile != null)

0 commit comments

Comments
 (0)