case 4715 - download file names checked
report/download and export/download (any download key), the log-file routes and backup download built a path from the file name in the URL with Path.Combine and no check, so on Windows a name with a folder part or an absolute path could reach files outside the folder (security review F21). New helper FileUtil.FullPathInFolderOrNull accepts only a bare file name that resolves directly inside the folder; TemporaryFileExists and BackupFileExists use it (the download routes already call them first), and so do both log routes. A refused name gets the same 404 as a missing file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -181,7 +181,7 @@ John's "next release" cases for AyaNova (exported 2026-10-24). Each is its own s
|
|||||||
| 4711 | JWT secret setting removed: the server generates its signing key once into `jwt-signing.key` in the data folder (4698 folded in) | F1. **Done** (placeholder-forged token 401; key survives restart; auth tests pass). Hosted setup script's secret step now does nothing; tidy later in `sockeye-client` |
|
| 4711 | JWT secret setting removed: the server generates its signing key once into `jwt-signing.key` in the data folder (4698 folded in) | F1. **Done** (placeholder-forged token 401; key survives restart; auth tests pass). Hosted setup script's secret step now does nothing; tidy later in `sockeye-client` |
|
||||||
| 4712 | Customer contact route allowlist + ownership checks | F13, F14, F15, F16, F17 |
|
| 4712 | Customer contact route allowlist + ownership checks | F13, F14, F15, F16, F17 |
|
||||||
| 4713 | Contact users limited to customer roles | F28 |
|
| 4713 | Contact users limited to customer roles | F28 |
|
||||||
| 4715 | Download file names checked (one helper) | F21 |
|
| 4715 | Download file names checked (one helper) | F21. **Done** (`FileUtil.FullPathInFolderOrNull` in temp/backup exists checks and both log routes; test `DownloadNames`) |
|
||||||
| 4716 | Pick-list search text as SQL parameters | F18 |
|
| 4716 | Pick-list search text as SQL parameters | F18 |
|
||||||
| 4717 | KPI interval allowlist | F26 |
|
| 4717 | KPI interval allowlist | F26 |
|
||||||
| 4718 | Report and translation import need create rights | F19, F24. **Done** (contact and Service 403, BizAdmin 202; test `ImportRights`) |
|
| 4718 | Report and translation import need create rights | F19, F24. **Done** (contact and Service 403, BizAdmin 202; test `ImportRights`) |
|
||||||
|
|||||||
@@ -194,7 +194,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes
|
|||||||
| F5 | **Critical** | **fixed in case 4710** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on |
|
| F5 | **Critical** | **fixed in case 4710** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on |
|
||||||
| F1 | Critical (installs that kept it) | **fixed in case 4711** | Placeholder JWT secret in the LAN/IIS and Linux server config templates |
|
| F1 | Critical (installs that kept it) | **fixed in case 4711** | Placeholder JWT secret in the LAN/IIS and Linux server config templates |
|
||||||
| F19 | Critical | **fixed in case 4718** | Any logged-in user, customer contacts included, can import report templates, whose JavaScript runs in the server's browser |
|
| F19 | Critical | **fixed in case 4718** | Any logged-in user, customer contacts included, can import report templates, whose JavaScript runs in the server's browser |
|
||||||
| F21 | Critical | candidate | Download file names from the URL are not checked: `report/download` and `export/download` may serve files outside the temp folder on Windows |
|
| F21 | Critical | **fixed in case 4715** | Download file names from the URL are not checked: `report/download` and `export/download` may serve files outside the temp folder on Windows |
|
||||||
| F18 | Critical | confirmed (code) | Pick-list search text is put into SQL without escaping; any logged-in user can reach it |
|
| F18 | Critical | confirmed (code) | Pick-list search text is put into SQL without escaping; any logged-in user can reach it |
|
||||||
| F26 | Critical | confirmed (code) | KPI `interval` criterion is put into SQL unchecked; reachable by staff users down to TechRestricted |
|
| F26 | Critical | confirmed (code) | KPI `interval` criterion is put into SQL unchecked; reachable by staff users down to TechRestricted |
|
||||||
| F13 | High | **confirmed** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id |
|
| F13 | High | **confirmed** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id |
|
||||||
|
|||||||
@@ -69,9 +69,10 @@ namespace AyaNova.Api.Controllers
|
|||||||
//stream the file contents into a json object and return
|
//stream the file contents into a json object and return
|
||||||
|
|
||||||
//build the full path from the log file name and defined path
|
//build the full path from the log file name and defined path
|
||||||
var logFilePath = System.IO.Path.Combine(ServerBootConfig.AYANOVA_LOG_PATH, logname);
|
//case 4715: only a bare file name inside the log folder
|
||||||
|
var logFilePath = FileUtil.FullPathInFolderOrNull(ServerBootConfig.AYANOVA_LOG_PATH, logname);
|
||||||
//does file exist?
|
//does file exist?
|
||||||
if (!System.IO.File.Exists(logFilePath))
|
if (logFilePath == null || !System.IO.File.Exists(logFilePath))
|
||||||
{
|
{
|
||||||
return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND));
|
return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND));
|
||||||
}
|
}
|
||||||
@@ -149,8 +150,9 @@ namespace AyaNova.Api.Controllers
|
|||||||
if (!Authorized.HasReadFullRole(user.Roles, AyaType.LogFile))
|
if (!Authorized.HasReadFullRole(user.Roles, AyaType.LogFile))
|
||||||
return StatusCode(403, new ApiNotAuthorizedResponse());
|
return StatusCode(403, new ApiNotAuthorizedResponse());
|
||||||
|
|
||||||
var logFilePath = System.IO.Path.Combine(ServerBootConfig.AYANOVA_LOG_PATH, logname);
|
//case 4715: only a bare file name inside the log folder
|
||||||
if (!System.IO.File.Exists(logFilePath))
|
var logFilePath = FileUtil.FullPathInFolderOrNull(ServerBootConfig.AYANOVA_LOG_PATH, logname);
|
||||||
|
if (logFilePath == null || !System.IO.File.Exists(logFilePath))
|
||||||
return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND));
|
return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND));
|
||||||
|
|
||||||
//Note: this works with new Nlog file locking for write so it must be opening it as read and shareable
|
//Note: this works with new Nlog file locking for write so it must be opening it as read and shareable
|
||||||
|
|||||||
@@ -116,12 +116,27 @@ namespace AyaNova.Util
|
|||||||
/// <returns>duh!</returns>
|
/// <returns>duh!</returns>
|
||||||
internal static bool TemporaryFileExists(string fileName)
|
internal static bool TemporaryFileExists(string fileName)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(fileName))
|
//case 4715: only a bare file name inside the temporary files folder
|
||||||
return false;
|
var FilePath = FullPathInFolderOrNull(TemporaryFilesFolder, fileName);
|
||||||
|
return FilePath != null && File.Exists(FilePath);
|
||||||
var FilePath = GetFullPathForTemporaryFile(fileName);
|
}
|
||||||
return File.Exists(FilePath);
|
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// case 4715: full path of a file name that came from a request, or null unless it is a bare file name
|
||||||
|
/// (no folder part, no "..") that resolves to a file directly inside the folder
|
||||||
|
/// </summary>
|
||||||
|
internal static string FullPathInFolderOrNull(string folder, string fileName)
|
||||||
|
{
|
||||||
|
if (string.IsNullOrWhiteSpace(folder) || string.IsNullOrWhiteSpace(fileName))
|
||||||
|
return null;
|
||||||
|
if (fileName != Path.GetFileName(fileName) || fileName.Contains("..") || fileName.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0)
|
||||||
|
return null;
|
||||||
|
var folderFullPath = Path.GetFullPath(folder);
|
||||||
|
var fileFullPath = Path.GetFullPath(Path.Combine(folderFullPath, fileName));
|
||||||
|
var comparison = OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
|
||||||
|
if (!string.Equals(Path.GetDirectoryName(fileFullPath), folderFullPath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), comparison))
|
||||||
|
return null;
|
||||||
|
return fileFullPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -275,12 +290,9 @@ namespace AyaNova.Util
|
|||||||
/// <returns>duh!</returns>
|
/// <returns>duh!</returns>
|
||||||
internal static bool BackupFileExists(string fileName)
|
internal static bool BackupFileExists(string fileName)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(fileName))
|
//case 4715: only a bare file name inside the backup files folder
|
||||||
return false;
|
var utilityFilePath = FullPathInFolderOrNull(ServerBootConfig.AYANOVA_BACKUP_FILES_PATH, fileName);
|
||||||
|
return utilityFilePath != null && File.Exists(utilityFilePath);
|
||||||
var utilityFilePath = GetFullPathForBackupFile(fileName);
|
|
||||||
return File.Exists(utilityFilePath);
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// /// <summary>
|
// /// <summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user