fix(security): validate scraped paths, BaseUrl, settings paths + atomic write
Five MEDIUM-tier security findings from the review. None were exploitable in
the single-user desktop threat model, but each is hardening for future hosts
(server / multi-user) and for the case of an upstream compromise.
* EventListingParserBase: scraped data-event-path values are now run through
IsSafeRelativePath before being concatenated into request URLs. Rejects
scheme://host/* patterns, leading slash/backslash (network-path traversal),
".." traversal, control characters (CRLF log forging), and path lengths
greater than 512.
* ScrapingModule.ResolveBaseAddress: configured Scraping:BaseUrl is now
validated through an https-only allow-list (marathonbet.by + subdomains).
Falls back to the default base URL when the value is missing, malformed,
off-host, or carries userinfo / query / fragment. Settings UI may write
arbitrary strings to this key — a typo or worse could otherwise re-point
every subsequent request at an unrelated host.
* JsonSettingsWriter.WriteRootAsync: temp-file write now FlushAsync +
Flush(flushToDisk: true) before the rename so a crash mid-write doesn't
leave a 0-byte appsettings.Local.json. Promote the rename from File.Move
(overwrite=true, not atomic on NTFS for cross-volume cases) to
File.Replace (atomic on NTFS, keeps a .bak copy of the previous file).
Falls back to File.Move when the destination doesn't exist yet.
* StoragePathValidator + Settings.razor wiring: DatabasePath and
ExportDirectory paths from the Settings page are validated before persist.
Rejects empty values, ".." traversal, control characters, and any path
that resolves outside AppContext.BaseDirectory. Adds 4 new RU+EN keys for
the validation messages.
* Logged scraper paths: covered transitively by the EventPath validation
above (the upstream of every logged {Path} value), so no separate
sanitization needed.
This commit is contained in:
@@ -81,6 +81,17 @@ public abstract class EventListingParserBase
|
||||
|
||||
var eventPath = row.GetAttribute("data-event-path");
|
||||
if (string.IsNullOrWhiteSpace(eventPath)) return null;
|
||||
if (!IsSafeRelativePath(eventPath))
|
||||
{
|
||||
// Defense in depth: data-event-path is concatenated into a
|
||||
// request URL by MarathonbetScraper. Reject anything that could
|
||||
// redirect the scraper to a different host, escape the base
|
||||
// directory, or carry control characters into a log line.
|
||||
Logger.LogWarning(
|
||||
"Rejecting event row with unsafe data-event-path value (eventId={EventId}).",
|
||||
eventIdRaw);
|
||||
return null;
|
||||
}
|
||||
|
||||
var eventName = row.GetAttribute("data-event-name") ?? string.Empty;
|
||||
|
||||
@@ -138,6 +149,33 @@ public abstract class EventListingParserBase
|
||||
return null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Validates that a scraped <c>data-event-path</c> value is safe to
|
||||
/// concatenate into the bookmaker URL. Rejects values that could:
|
||||
/// (a) redirect the scraper to a different host (network-path "//host/x"
|
||||
/// or absolute "scheme://host/..."); (b) traverse out of the base
|
||||
/// directory (".."); (c) inject CRLF / control characters into log
|
||||
/// lines or HTTP headers.
|
||||
/// </summary>
|
||||
private static bool IsSafeRelativePath(string path)
|
||||
{
|
||||
if (path.Length is 0 or > 512)
|
||||
return false;
|
||||
if (path[0] is '/' or '\\')
|
||||
return false;
|
||||
if (path.Contains("://", StringComparison.Ordinal))
|
||||
return false;
|
||||
if (path.Contains(".."))
|
||||
return false;
|
||||
|
||||
foreach (var ch in path)
|
||||
{
|
||||
if (ch < 0x20 || ch == 0x7F)
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private static (string country, string league, string category) ParseEventPath(string path)
|
||||
{
|
||||
// Path example:
|
||||
|
||||
@@ -51,7 +51,7 @@ public static class ScrapingModule
|
||||
.AddHttpClient("marathonbet", (sp, client) =>
|
||||
{
|
||||
var opts = sp.GetRequiredService<IOptions<ScrapingOptions>>().Value;
|
||||
client.BaseAddress = new Uri(opts.BaseUrl);
|
||||
client.BaseAddress = ResolveBaseAddress(opts.BaseUrl);
|
||||
client.Timeout = Timeout.InfiniteTimeSpan; // Polly timeout manages per-attempt
|
||||
})
|
||||
.AddHttpMessageHandler<UserAgentRotatorHandler>()
|
||||
@@ -125,4 +125,44 @@ public static class ScrapingModule
|
||||
|
||||
return services;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Validates a configured <c>Scraping:BaseUrl</c> and returns the parsed
|
||||
/// <see cref="Uri"/>. Falls back to the default
|
||||
/// <c>https://www.marathonbet.by</c> when the configured value is missing
|
||||
/// or fails the safety checks (https only, host on the marathonbet.by
|
||||
/// allow-list, no userinfo / fragment / query) — settings UI may write
|
||||
/// arbitrary strings here, and we don't want a typo (or worse) to point
|
||||
/// the scraper at an unrelated host.
|
||||
/// </summary>
|
||||
private static Uri ResolveBaseAddress(string? configured)
|
||||
{
|
||||
const string defaultBase = "https://www.marathonbet.by";
|
||||
|
||||
if (string.IsNullOrWhiteSpace(configured))
|
||||
return new Uri(defaultBase);
|
||||
|
||||
if (!Uri.TryCreate(configured, UriKind.Absolute, out var parsed))
|
||||
return new Uri(defaultBase);
|
||||
|
||||
if (!string.Equals(parsed.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))
|
||||
return new Uri(defaultBase);
|
||||
|
||||
// Allow-list — only marathonbet.by and its subdomains. Future bookmaker
|
||||
// support should be opt-in via additional ScrapingOptions, not by
|
||||
// letting any URL through this gate.
|
||||
var host = parsed.Host;
|
||||
var isAllowed =
|
||||
host.Equals("marathonbet.by", StringComparison.OrdinalIgnoreCase) ||
|
||||
host.EndsWith(".marathonbet.by", StringComparison.OrdinalIgnoreCase);
|
||||
if (!isAllowed)
|
||||
return new Uri(defaultBase);
|
||||
|
||||
if (!string.IsNullOrEmpty(parsed.UserInfo) ||
|
||||
!string.IsNullOrEmpty(parsed.Fragment) ||
|
||||
!string.IsNullOrEmpty(parsed.Query))
|
||||
return new Uri(defaultBase);
|
||||
|
||||
return parsed;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -230,6 +230,18 @@
|
||||
|
||||
private async Task SaveSectionAsync<T>(string section, T payload) where T : class
|
||||
{
|
||||
// Section-level validation — fails fast before disk write so the
|
||||
// user sees the snackbar immediately and the JSON file isn't touched.
|
||||
if (payload is StorageOptions storage)
|
||||
{
|
||||
var errorKey = StoragePathValidator.Validate(storage);
|
||||
if (errorKey is not null)
|
||||
{
|
||||
Snackbar.Add(L[errorKey], Severity.Error);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
var confirmed = await ConfirmAsync();
|
||||
if (!confirmed)
|
||||
{
|
||||
|
||||
@@ -125,6 +125,10 @@
|
||||
<data name="Settings.Storage.DatabasePath"><value>SQLite path</value></data>
|
||||
<data name="Settings.Storage.ExportDirectory"><value>Export directory</value></data>
|
||||
<data name="Settings.Storage.SnapshotRetentionDays"><value>Snapshot retention (days)</value></data>
|
||||
<data name="Settings.Storage.Validation.DatabasePathRequired"><value>SQLite path is required.</value></data>
|
||||
<data name="Settings.Storage.Validation.ExportDirectoryRequired"><value>Export directory is required.</value></data>
|
||||
<data name="Settings.Storage.Validation.DatabasePathInvalid"><value>SQLite path must stay inside the application directory and may not contain "..".</value></data>
|
||||
<data name="Settings.Storage.Validation.ExportDirectoryInvalid"><value>Export directory must stay inside the application directory and may not contain "..".</value></data>
|
||||
|
||||
<data name="Settings.Anomaly.SuspensionGapSeconds"><value>Suspension window (sec)</value></data>
|
||||
<data name="Settings.Anomaly.OddsFlipThreshold"><value>Flip threshold (Δ probability)</value></data>
|
||||
|
||||
@@ -132,6 +132,10 @@
|
||||
<data name="Settings.Storage.DatabasePath"><value>Путь к SQLite</value></data>
|
||||
<data name="Settings.Storage.ExportDirectory"><value>Каталог экспорта</value></data>
|
||||
<data name="Settings.Storage.SnapshotRetentionDays"><value>Хранить снимки (дней)</value></data>
|
||||
<data name="Settings.Storage.Validation.DatabasePathRequired"><value>Путь к SQLite обязателен.</value></data>
|
||||
<data name="Settings.Storage.Validation.ExportDirectoryRequired"><value>Каталог экспорта обязателен.</value></data>
|
||||
<data name="Settings.Storage.Validation.DatabasePathInvalid"><value>Путь к SQLite должен быть внутри папки приложения и не содержать «..».</value></data>
|
||||
<data name="Settings.Storage.Validation.ExportDirectoryInvalid"><value>Каталог экспорта должен быть внутри папки приложения и не содержать «..».</value></data>
|
||||
|
||||
<!-- Settings — Anomaly -->
|
||||
<data name="Settings.Anomaly.SuspensionGapSeconds"><value>Окно «заморозки» (сек)</value></data>
|
||||
|
||||
@@ -117,13 +117,36 @@ public sealed class JsonSettingsWriter : ISettingsWriter
|
||||
}
|
||||
|
||||
var tempPath = _filePath + ".tmp";
|
||||
await using (var stream = File.Create(tempPath))
|
||||
|
||||
// Write the temp file and force the buffer to disk BEFORE renaming.
|
||||
// Without Flush(true) a crash between the write and the rename leaves
|
||||
// a 0-byte appsettings.Local.json on the next boot.
|
||||
await using (var stream = new FileStream(
|
||||
tempPath,
|
||||
FileMode.Create,
|
||||
FileAccess.Write,
|
||||
FileShare.None,
|
||||
bufferSize: 4096,
|
||||
useAsync: true))
|
||||
{
|
||||
await JsonSerializer.SerializeAsync(stream, root, WriteOptions, cancellationToken).ConfigureAwait(false);
|
||||
await stream.FlushAsync(cancellationToken).ConfigureAwait(false);
|
||||
stream.Flush(flushToDisk: true);
|
||||
}
|
||||
|
||||
// Atomic rename — survives crashes mid-write.
|
||||
File.Move(tempPath, _filePath, overwrite: true);
|
||||
// File.Replace is atomic on NTFS and keeps a .bak copy of the prior
|
||||
// contents in case the next boot fails to start with the new settings.
|
||||
// Falls back to File.Move when the destination doesn't exist yet
|
||||
// (Replace requires the destination present; Move handles first-write).
|
||||
if (File.Exists(_filePath))
|
||||
{
|
||||
var backupPath = _filePath + ".bak";
|
||||
File.Replace(tempPath, _filePath, backupPath, ignoreMetadataErrors: true);
|
||||
}
|
||||
else
|
||||
{
|
||||
File.Move(tempPath, _filePath, overwrite: false);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>For tests: reads the persisted JSON object back.</summary>
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
using Marathon.Application.Storage;
|
||||
|
||||
namespace Marathon.UI.Services;
|
||||
|
||||
/// <summary>
|
||||
/// Validates user-supplied <see cref="StorageOptions"/> values from the
|
||||
/// Settings page before they're persisted to <c>appsettings.Local.json</c>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// On a single-user desktop build this is "user shoots own foot" hardening.
|
||||
/// On a future ASP.NET host with the same RCL it becomes a real boundary —
|
||||
/// neither the SQLite provider nor the Excel exporter validate paths
|
||||
/// themselves.
|
||||
/// </remarks>
|
||||
public static class StoragePathValidator
|
||||
{
|
||||
/// <summary>
|
||||
/// Returns a localizable error key when <paramref name="options"/> is
|
||||
/// invalid, or <c>null</c> when it's safe to persist.
|
||||
/// Validation rules:
|
||||
/// <list type="bullet">
|
||||
/// <item>Both fields must be non-empty.</item>
|
||||
/// <item>No control characters or characters Windows rejects in paths.</item>
|
||||
/// <item>No <c>".."</c> traversal segments.</item>
|
||||
/// <item>Paths must resolve under <see cref="AppContext.BaseDirectory"/> —
|
||||
/// either as relative paths (anchored on the working directory) or
|
||||
/// as absolute paths whose <see cref="Path.GetFullPath(string)"/>
|
||||
/// starts with the base directory's full path.</item>
|
||||
/// </list>
|
||||
/// </summary>
|
||||
public static string? Validate(StorageOptions options)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(options);
|
||||
|
||||
if (string.IsNullOrWhiteSpace(options.DatabasePath))
|
||||
return "Settings.Storage.Validation.DatabasePathRequired";
|
||||
if (string.IsNullOrWhiteSpace(options.ExportDirectory))
|
||||
return "Settings.Storage.Validation.ExportDirectoryRequired";
|
||||
|
||||
if (!IsAcceptablePath(options.DatabasePath))
|
||||
return "Settings.Storage.Validation.DatabasePathInvalid";
|
||||
if (!IsAcceptablePath(options.ExportDirectory))
|
||||
return "Settings.Storage.Validation.ExportDirectoryInvalid";
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
private static bool IsAcceptablePath(string raw)
|
||||
{
|
||||
if (raw.Contains(".."))
|
||||
return false;
|
||||
if (raw.IndexOfAny(Path.GetInvalidPathChars()) >= 0)
|
||||
return false;
|
||||
|
||||
try
|
||||
{
|
||||
var baseDir = Path.GetFullPath(AppContext.BaseDirectory);
|
||||
var resolved = Path.GetFullPath(
|
||||
Path.IsPathRooted(raw) ? raw : Path.Combine(AppContext.BaseDirectory, raw));
|
||||
|
||||
// Allow same path or any descendant. Use OrdinalIgnoreCase on
|
||||
// Windows; case-sensitive elsewhere.
|
||||
var comparison = OperatingSystem.IsWindows()
|
||||
? StringComparison.OrdinalIgnoreCase
|
||||
: StringComparison.Ordinal;
|
||||
|
||||
return resolved.StartsWith(
|
||||
baseDir.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar,
|
||||
comparison)
|
||||
|| string.Equals(resolved, baseDir, comparison);
|
||||
}
|
||||
catch (ArgumentException)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
catch (PathTooLongException)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
catch (NotSupportedException)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user