fix(epub): resolve PR #65 review comments for EPUB image rendering, path traversal validation, and security sanitization

This commit is contained in:
2026-06-01 18:03:39 +02:00
parent 9c32d28e93
commit 11b08cf7ba
10 changed files with 179 additions and 16 deletions
@@ -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();
}
+4
View File
@@ -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>();
@@ -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))
{
@@ -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);
@@ -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 { }
}
}
}
@@ -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;
}
@@ -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
{
+3
View File
@@ -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>();
+20 -3
View File
@@ -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",
@@ -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();