From 11b08cf7bab368c892ba9eb9f31d811a053a4825 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Jasi=C5=84ski?= <jasins.marek@gmail.com>
Date: Mon, 1 Jun 2026 18:03:39 +0200
Subject: [PATCH] fix(epub): resolve PR #65 review comments for EPUB image
 rendering, path traversal validation, and security sanitization

---
 .../Services/EpubReaderService.cs             | 47 ++++++++--
 src/NexusReader.Maui/MauiProgram.cs           |  4 +
 .../Pages/Account/Login.razor                 |  6 +-
 .../Pages/Account/Register.razor              |  4 +-
 .../Pages/SerilogDemo.razor                   |  5 ++
 .../Services/FeatureSettings.cs               | 11 +++
 .../Services/ReaderStateService.cs            |  2 +
 src/NexusReader.Web.Client/Program.cs         |  3 +
 src/NexusReader.Web/Program.cs                | 23 ++++-
 .../Services/EpubReaderServiceTests.cs        | 90 +++++++++++++++++++
 10 files changed, 179 insertions(+), 16 deletions(-)
 create mode 100644 src/NexusReader.UI.Shared/Services/FeatureSettings.cs

diff --git a/src/NexusReader.Infrastructure/Services/EpubReaderService.cs b/src/NexusReader.Infrastructure/Services/EpubReaderService.cs
index 44b5d41..6079d42 100644
--- a/src/NexusReader.Infrastructure/Services/EpubReaderService.cs
+++ b/src/NexusReader.Infrastructure/Services/EpubReaderService.cs
@@ -18,6 +18,16 @@ public class EpubReaderService : IEpubReader
     private readonly ILogger<EpubReaderService> _logger;
     private const int WordThreshold = 1000;
 
+    private static readonly Regex ImageTagRegex = new(@"<img\b(?<before>[^>]*?\bsrc=[""'])(?<src>[^""']*?)(?<after>[""'][^>]*?>)", RegexOptions.IgnoreCase | RegexOptions.Compiled);
+    private static readonly Regex BodyMatchRegex = new(@"<body\b[^>]*>(.*?)</body>", RegexOptions.IgnoreCase | RegexOptions.Singleline | RegexOptions.Compiled);
+    private static readonly Regex ParagraphMatchRegex = new(@"<(p|h[1-6]|ul|ol|blockquote|pre)\b[^>]*>.*?</\1>|<hr\b[^>]*>|<img\b[^>]*>", RegexOptions.IgnoreCase | RegexOptions.Singleline | RegexOptions.Compiled);
+    private static readonly Regex StyleScriptRegex = new(@"<(style|script)\b[^>]*>.*?</\1>", RegexOptions.IgnoreCase | RegexOptions.Singleline | RegexOptions.Compiled);
+    private static readonly Regex WhitelistTagsRegex = new(@"<(?!/?(b|i|strong|em|h[1-6]|p|ul|ol|li|blockquote|pre|code|br|hr|img)\b)[^>]+>", RegexOptions.IgnoreCase | RegexOptions.Compiled);
+    private static readonly Regex StripAttributesRegex = new(@"<(b|i|strong|em|h[1-6]|p|ul|ol|li|blockquote|pre|code|br|hr)\b[^>]*>", RegexOptions.IgnoreCase | RegexOptions.Compiled);
+    private static readonly Regex ImgTagSanitizerRegex = new(@"<img\b[^>]*>", RegexOptions.IgnoreCase | RegexOptions.Compiled);
+    private static readonly Regex SrcAttributeRegex = new(@"\bsrc=[""'](?<src>[^""']*)[""']", RegexOptions.IgnoreCase | RegexOptions.Compiled);
+    private static readonly Regex AltAttributeRegex = new(@"\balt=[""'](?<alt>[^""']*)[""']", RegexOptions.IgnoreCase | RegexOptions.Compiled);
+
     public EpubReaderService(
         IDbContextFactory<AppDbContext> dbContextFactory,
         ILogger<EpubReaderService> logger)
@@ -174,7 +184,13 @@ public class EpubReaderService : IEpubReader
 
             using var bookRef = await EpubReader.OpenBookAsync(fullPath);
             
-            var decodedPath = System.Net.WebUtility.UrlDecode(resourcePath).Replace('\\', '/').TrimStart('/');
+            var decodedPath = System.Net.WebUtility.UrlDecode(resourcePath);
+            if (decodedPath.Contains("..") || decodedPath.Contains(":") || decodedPath.StartsWith("/") || decodedPath.StartsWith("\\"))
+            {
+                return Result.Fail("Invalid resource path.");
+            }
+
+            decodedPath = decodedPath.Replace('\\', '/').TrimStart('/');
 
             EpubLocalContentFileRef? targetFile = null;
             if (bookRef.Content?.AllFiles?.Local != null)
@@ -220,11 +236,15 @@ public class EpubReaderService : IEpubReader
     {
         if (string.IsNullOrEmpty(html)) return html;
 
-        var imgRegex = new Regex(@"<img\b(?<before>[^>]*?\bsrc=[""'])(?<src>[^""']*?)(?<after>[""'][^>]*?>)", RegexOptions.IgnoreCase);
-        return imgRegex.Replace(html, match =>
+        return ImageTagRegex.Replace(html, match =>
         {
             var rawSrc = match.Groups["src"].Value;
             
+            if (rawSrc.StartsWith("javascript:", StringComparison.OrdinalIgnoreCase))
+            {
+                return ""; // Completely block script execution in image src
+            }
+
             if (rawSrc.StartsWith("http://", StringComparison.OrdinalIgnoreCase) ||
                 rawSrc.StartsWith("https://", StringComparison.OrdinalIgnoreCase) ||
                 rawSrc.StartsWith("data:", StringComparison.OrdinalIgnoreCase))
@@ -274,11 +294,11 @@ public class EpubReaderService : IEpubReader
 
     private static List<string> ExtractParagraphs(string html)
     {
-        var bodyMatch = Regex.Match(html, @"<body\b[^>]*>(.*?)</body>", RegexOptions.IgnoreCase | RegexOptions.Singleline);
+        var bodyMatch = BodyMatchRegex.Match(html);
         var content = bodyMatch.Success ? bodyMatch.Groups[1].Value : html;
 
         var paragraphs = new List<string>();
-        var matches = Regex.Matches(content, @"<(p|h[1-6]|ul|ol|blockquote|pre)\b[^>]*>.*?</\1>|<hr\b[^>]*>|<img\b[^>]*>", RegexOptions.IgnoreCase | RegexOptions.Singleline);
+        var matches = ParagraphMatchRegex.Matches(content);
 
         foreach (Match match in matches)
         {
@@ -295,9 +315,20 @@ public class EpubReaderService : IEpubReader
 
     private static string SanitizeParagraph(string html)
     {
-        var clean = Regex.Replace(html, @"<(style|script)\b[^>]*>.*?</\1>", "", RegexOptions.IgnoreCase | RegexOptions.Singleline);
-        clean = Regex.Replace(clean, @"<(?!/?(b|i|strong|em|h[1-6]|p|ul|ol|li|blockquote|pre|code|br|hr|img)\b)[^>]+>", "", RegexOptions.IgnoreCase);
-        clean = Regex.Replace(clean, @"<(b|i|strong|em|h[1-6]|p|ul|ol|li|blockquote|pre|code|br|hr)\b[^>]*>", "<$1>", RegexOptions.IgnoreCase);
+        var clean = StyleScriptRegex.Replace(html, "");
+        clean = WhitelistTagsRegex.Replace(clean, "");
+        clean = StripAttributesRegex.Replace(clean, "<$1>");
+
+        // Securely sanitize img tags by keeping ONLY src and alt attributes to prevent XSS (onerror, onload, style, etc.)
+        clean = ImgTagSanitizerRegex.Replace(clean, m =>
+        {
+            var srcMatch = SrcAttributeRegex.Match(m.Value);
+            var altMatch = AltAttributeRegex.Match(m.Value);
+            var srcAttr = srcMatch.Success ? $" src=\"{srcMatch.Groups["src"].Value}\"" : "";
+            var altAttr = altMatch.Success ? $" alt=\"{altMatch.Groups["alt"].Value}\"" : "";
+            return $"<img{srcAttr}{altAttr} />";
+        });
+
         clean = System.Net.WebUtility.HtmlDecode(clean);
         return clean.Trim();
     }
diff --git a/src/NexusReader.Maui/MauiProgram.cs b/src/NexusReader.Maui/MauiProgram.cs
index a2a7733..a4b2c6c 100644
--- a/src/NexusReader.Maui/MauiProgram.cs
+++ b/src/NexusReader.Maui/MauiProgram.cs
@@ -63,6 +63,10 @@ public static class MauiProgram
 			builder.Services.AddScoped(sp => sp.GetRequiredService<IHttpClientFactory>().CreateClient("NexusAPI"));
 
 			// UI State
+			// Feature settings (avoiding direct raw IConfiguration injection in client pages)
+			var featureSettings = builder.Configuration.GetSection("Features").Get<FeatureSettings>() ?? new FeatureSettings();
+			builder.Services.AddSingleton(featureSettings);
+
 			builder.Services.AddScoped<IThemeService, ThemeService>();
 			builder.Services.AddScoped<IFocusModeService, FocusModeService>();
 			builder.Services.AddScoped<IQuizStateService, QuizStateService>();
diff --git a/src/NexusReader.UI.Shared/Pages/Account/Login.razor b/src/NexusReader.UI.Shared/Pages/Account/Login.razor
index 703382a..5295a1f 100644
--- a/src/NexusReader.UI.Shared/Pages/Account/Login.razor
+++ b/src/NexusReader.UI.Shared/Pages/Account/Login.razor
@@ -7,7 +7,7 @@
 @inject IIdentityService IdentityService
 @inject NavigationManager NavigationManager
 @inject IJSRuntime JS
-@inject IConfiguration Configuration
+@inject FeatureSettings FeatureSettings
 
 <div class="login-page-container">
     <div class="mesh-bg"></div>
@@ -126,8 +126,8 @@
 
     protected override async Task OnInitializedAsync()
     {
-        _allowRegistration = Configuration.GetValue<bool?>("Features:AllowRegistration") ?? true;
-        _allowPasswordReset = Configuration.GetValue<bool?>("Features:AllowPasswordReset") ?? true;
+        _allowRegistration = FeatureSettings.AllowRegistration;
+        _allowPasswordReset = FeatureSettings.AllowPasswordReset;
 
         if (!string.IsNullOrEmpty(ErrorCode))
         {
diff --git a/src/NexusReader.UI.Shared/Pages/Account/Register.razor b/src/NexusReader.UI.Shared/Pages/Account/Register.razor
index 95534c8..b4b18bb 100644
--- a/src/NexusReader.UI.Shared/Pages/Account/Register.razor
+++ b/src/NexusReader.UI.Shared/Pages/Account/Register.razor
@@ -7,7 +7,7 @@
 @inject IIdentityService IdentityService
 @inject NavigationManager NavigationManager
 @inject IJSRuntime JS
-@inject IConfiguration Configuration
+@inject FeatureSettings FeatureSettings
 
 <div class="login-page-container">
     <div class="mesh-bg"></div>
@@ -84,7 +84,7 @@
 
     protected override void OnInitialized()
     {
-        var allowRegistration = Configuration.GetValue<bool?>("Features:AllowRegistration") ?? true;
+        var allowRegistration = FeatureSettings.AllowRegistration;
         if (!allowRegistration)
         {
             NavigationManager.NavigateTo("/account/login?error=RegistrationDisabled", replace: true);
diff --git a/src/NexusReader.UI.Shared/Pages/SerilogDemo.razor b/src/NexusReader.UI.Shared/Pages/SerilogDemo.razor
index fde0f5d..8e2efbb 100644
--- a/src/NexusReader.UI.Shared/Pages/SerilogDemo.razor
+++ b/src/NexusReader.UI.Shared/Pages/SerilogDemo.razor
@@ -151,6 +151,11 @@ else
         catch (Exception ex)
         {
             Logger.LogError(ex, "Simulated runtime JS Exception triggered and captured in Blazor UI");
+            try
+            {
+                await JSRuntime.InvokeVoidAsync("console.error", $"Simulated JS Exception: {ex.Message}");
+            }
+            catch { }
         }
     }
 }
diff --git a/src/NexusReader.UI.Shared/Services/FeatureSettings.cs b/src/NexusReader.UI.Shared/Services/FeatureSettings.cs
new file mode 100644
index 0000000..8a79ba9
--- /dev/null
+++ b/src/NexusReader.UI.Shared/Services/FeatureSettings.cs
@@ -0,0 +1,11 @@
+namespace NexusReader.UI.Shared.Services;
+
+/// <summary>
+/// Strongly-typed feature settings for the client UI layer.
+/// Used to decouple the UI from raw IConfiguration to prevent exposure of sensitive settings.
+/// </summary>
+public class FeatureSettings
+{
+    public bool AllowRegistration { get; set; } = true;
+    public bool AllowPasswordReset { get; set; } = true;
+}
diff --git a/src/NexusReader.UI.Shared/Services/ReaderStateService.cs b/src/NexusReader.UI.Shared/Services/ReaderStateService.cs
index 4906759..3759fd6 100644
--- a/src/NexusReader.UI.Shared/Services/ReaderStateService.cs
+++ b/src/NexusReader.UI.Shared/Services/ReaderStateService.cs
@@ -4,6 +4,8 @@ namespace NexusReader.UI.Shared.Services;
 
 /// <summary>
 /// Thread-safe implementation of IReaderStateService.
+/// Thread safety is ensured via lock-guarded property getters/setters.
+/// UI updates originating from the JS event loop (via JSInvokable) are synchronized at Blazor's InvokeAsync(StateHasChanged) render boundary.
 /// </summary>
 public sealed class ReaderStateService : IReaderStateService
 {
diff --git a/src/NexusReader.Web.Client/Program.cs b/src/NexusReader.Web.Client/Program.cs
index 68a7479..bac3723 100644
--- a/src/NexusReader.Web.Client/Program.cs
+++ b/src/NexusReader.Web.Client/Program.cs
@@ -18,6 +18,9 @@ var builder = WebAssemblyHostBuilder.CreateDefault(args);
 builder.Services.AddScoped<IPlatformService, WebPlatformService>();
 builder.Services.AddScoped<INativeStorageService, WebStorageService>();
 builder.Services.AddScoped<IThemeService, ThemeService>();
+// Feature settings (avoiding direct raw IConfiguration injection in client pages)
+var featureSettings = builder.Configuration.GetSection("Features").Get<FeatureSettings>() ?? new FeatureSettings();
+builder.Services.AddSingleton(featureSettings);
 builder.Services.AddScoped<IQuizStateService, QuizStateService>();
 builder.Services.AddScoped<IFocusModeService, FocusModeService>();
 builder.Services.AddScoped<IReaderNavigationService, ReaderNavigationService>();
diff --git a/src/NexusReader.Web/Program.cs b/src/NexusReader.Web/Program.cs
index 4ae273d..38c8006 100644
--- a/src/NexusReader.Web/Program.cs
+++ b/src/NexusReader.Web/Program.cs
@@ -48,6 +48,9 @@ builder.Services.AddHttpContextAccessor();
 builder.Services.AddScoped<IPlatformService, WebPlatformService>();
 builder.Services.AddScoped<INativeStorageService, NexusReader.Web.Services.NativeStorageService>();
 builder.Services.AddScoped<IThemeService, ThemeService>();
+// Feature settings (avoiding direct raw IConfiguration injection in client pages)
+var featureSettings = builder.Configuration.GetSection("Features").Get<FeatureSettings>() ?? new FeatureSettings();
+builder.Services.AddSingleton(featureSettings);
 builder.Services.AddScoped<IQuizStateService, QuizStateService>();
 builder.Services.AddScoped<IFocusModeService, FocusModeService>();
 builder.Services.AddScoped<IReaderNavigationService, ReaderNavigationService>();
@@ -298,14 +301,28 @@ app.MapGet("/api/epub/{ebookId:guid}/{index:int}", async (Guid ebookId, int inde
 }).RequireAuthorization();
 
 // API endpoint for WASM client/browser to fetch EPUB static resources (images, etc.)
-app.MapGet("/api/epub/{ebookId:guid}/resource", async (Guid ebookId, string path, IEpubReader epubService, ClaimsPrincipal user, CancellationToken cancellationToken) =>
+app.MapGet("/api/epub/{ebookId:guid}/resource", async (Guid ebookId, string path, IEpubReader epubService, ClaimsPrincipal user, HttpContext httpContext, CancellationToken cancellationToken) =>
 {
+    if (string.IsNullOrEmpty(path))
+    {
+        return Results.BadRequest("Path parameter is required.");
+    }
+
+    var decodedPath = Uri.UnescapeDataString(path);
+    if (decodedPath.Contains("..") || decodedPath.Contains(":") || decodedPath.StartsWith("/") || decodedPath.StartsWith("\\"))
+    {
+        return Results.BadRequest("Invalid resource path.");
+    }
+
     var userId = user.FindFirstValue(ClaimTypes.NameIdentifier);
-    var result = await epubService.GetEpubResourceAsync(ebookId, path, userId, cancellationToken);
+    var result = await epubService.GetEpubResourceAsync(ebookId, decodedPath, userId, cancellationToken);
     
     if (result.IsSuccess)
     {
-        var ext = Path.GetExtension(path).ToLowerInvariant();
+        // Serve with client-side caching to avoid redundant roundtrips on chapter navigation
+        httpContext.Response.Headers.CacheControl = "public, max-age=86400";
+
+        var ext = Path.GetExtension(decodedPath).ToLowerInvariant();
         var contentType = ext switch
         {
             ".jpg" or ".jpeg" => "image/jpeg",
diff --git a/tests/NexusReader.Application.Tests/Services/EpubReaderServiceTests.cs b/tests/NexusReader.Application.Tests/Services/EpubReaderServiceTests.cs
index e01f044..81609bf 100644
--- a/tests/NexusReader.Application.Tests/Services/EpubReaderServiceTests.cs
+++ b/tests/NexusReader.Application.Tests/Services/EpubReaderServiceTests.cs
@@ -177,6 +177,96 @@ public class EpubReaderServiceTests : IDisposable
         }
     }
 
+    [Fact]
+    public void SanitizeParagraph_StripsUnsafeAttributesFromImgTags()
+    {
+        // Arrange
+        var method = typeof(EpubReaderService).GetMethod("SanitizeParagraph", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
+        method.Should().NotBeNull();
+
+        var input = "<img src=\"images/cover.jpg\" alt=\"Cover Image\" onerror=\"alert(1)\" onload=\"evil()\" style=\"color:red\" class=\"img-responsive\" />";
+
+        // Act
+        var result = (string)method.Invoke(null, new object[] { input });
+
+        // Assert
+        result.Should().NotContain("onerror");
+        result.Should().NotContain("onload");
+        result.Should().NotContain("style");
+        result.Should().NotContain("class");
+        result.Should().Contain("src=\"images/cover.jpg\"");
+        result.Should().Contain("alt=\"Cover Image\"");
+    }
+
+    [Fact]
+    public void RewriteImageUrls_BlocksJavaScriptScheme()
+    {
+        // Arrange
+        var method = typeof(EpubReaderService).GetMethod("RewriteImageUrls", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
+        method.Should().NotBeNull();
+
+        var input = "<img src=\"javascript:alert(1)\" />";
+        var ebookId = Guid.NewGuid();
+
+        // Act
+        var result = (string)method.Invoke(null, new object[] { input, ebookId, "OEBPS/chapter1.xhtml" });
+
+        // Assert
+        result.Should().NotContain("javascript:alert(1)");
+    }
+
+    [Fact]
+    public async Task GetEpubResourceAsync_RejectsInvalidResourcePaths()
+    {
+        // Arrange
+        var ebookId = Guid.NewGuid();
+        var userId = "test-user-id";
+
+        using (var context = new AppDbContext(_contextOptions))
+        {
+            var user = new NexusUser
+            {
+                Id = userId,
+                UserName = "testuser",
+                Email = "test@nexus.com",
+                TenantId = "tenant-123",
+                SubscriptionPlanId = 1
+            };
+            context.Users.Add(user);
+
+            var author = new Author { Id = 10, Name = "Giorgio Vasari" };
+            context.Authors.Add(author);
+
+            var ebook = new Ebook
+            {
+                Id = ebookId,
+                UserId = userId,
+                Title = "Test Book",
+                AuthorId = author.Id,
+                FilePath = "assets/book.epub",
+                AddedDate = DateTime.UtcNow,
+                LastReadDate = DateTime.UtcNow,
+                Progress = 0,
+                LastChapter = "Introduction"
+            };
+            context.Ebooks.Add(ebook);
+            await context.SaveChangesAsync();
+        }
+
+        var service = new EpubReaderService(_dbContextFactoryMock.Object, _loggerMock.Object);
+
+        // Act
+        var traversalResult = await service.GetEpubResourceAsync(ebookId, "../../appsettings.json", userId);
+        var colonResult = await service.GetEpubResourceAsync(ebookId, "C:\\windows\\win.ini", userId);
+
+        // Assert
+        traversalResult.IsSuccess.Should().BeFalse();
+        traversalResult.Errors.First().Message.Should().Contain("Invalid resource path");
+
+        colonResult.IsSuccess.Should().BeFalse();
+        colonResult.Errors.First().Message.Should().Contain("Invalid resource path");
+    }
+
     public void Dispose()
     {
         _connection.Close();