mirror of
https://github.com/Radarr/Radarr
synced 2026-01-24 08:23:54 +01:00
fix(security): address P3 vulnerabilities and add mitigations
Security fixes: - XXE prevention: disable XmlResolver in UTorrentProxy.cs (#42) - Path traversal: validate paths in LogFileController.cs (#44) - Path traversal: validate paths in MediaCoverController.cs (#44) - ReDoS mitigation: add 5s timeout to user regex patterns Documentation: - CORS: document security rationale in Startup.cs (#43) Closes #42, #43, #44 Related: #59, #60, #61 (SonarCloud triage - GitHub alerts now at 0 open) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
f7fca51da7
commit
019f0862b3
5 changed files with 26 additions and 3 deletions
|
|
@ -263,6 +263,7 @@ private void AuthenticateClient(HttpRequestBuilder requestBuilder, UTorrentSetti
|
|||
_logger.Debug("uTorrent authentication succeeded.");
|
||||
|
||||
var xmlDoc = new System.Xml.XmlDocument();
|
||||
xmlDoc.XmlResolver = null; // Disable external entity resolution (XXE prevention)
|
||||
xmlDoc.LoadXml(response.Content);
|
||||
|
||||
authToken = xmlDoc.FirstChild.FirstChild.InnerText;
|
||||
|
|
|
|||
|
|
@ -26,7 +26,8 @@ public static Regex CreateRegex(string pattern, string modifiers)
|
|||
var options = GetOptions(modifiers);
|
||||
|
||||
// For now we simply expect the pattern to be .net compliant. We should probably check and reject perl-specific constructs.
|
||||
return new Regex(pattern, options | RegexOptions.Compiled);
|
||||
// Use timeout to mitigate ReDoS attacks from malicious patterns
|
||||
return new Regex(pattern, options | RegexOptions.Compiled, TimeSpan.FromSeconds(5));
|
||||
}
|
||||
|
||||
private static RegexOptions GetOptions(string modifiers)
|
||||
|
|
|
|||
|
|
@ -73,6 +73,10 @@ public void ConfigureServices(IServiceCollection services)
|
|||
|
||||
services.AddResponseCompression(options => options.EnableForHttps = true);
|
||||
|
||||
// CORS is permissive because:
|
||||
// 1. All API endpoints require authentication (API key or session)
|
||||
// 2. Single-user self-hosted model - no cross-user attack surface
|
||||
// 3. Restrictive CORS would break mobile apps and browser extensions
|
||||
services.AddCors(options =>
|
||||
{
|
||||
options.AddPolicy(VersionedApiControllerAttribute.API_CORS_POLICY,
|
||||
|
|
|
|||
|
|
@ -30,7 +30,17 @@ protected override IEnumerable<string> GetLogFiles()
|
|||
|
||||
protected override string GetLogFilePath(string filename)
|
||||
{
|
||||
return Path.Combine(_appFolderInfo.GetLogFolder(), filename);
|
||||
var logFolder = Path.GetFullPath(_appFolderInfo.GetLogFolder());
|
||||
var filePath = Path.GetFullPath(Path.Combine(logFolder, filename));
|
||||
|
||||
// Prevent path traversal - ensure path stays within log folder
|
||||
if (!filePath.StartsWith(logFolder + Path.DirectorySeparatorChar) &&
|
||||
!filePath.Equals(logFolder, global::System.StringComparison.Ordinal))
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
return filePath;
|
||||
}
|
||||
|
||||
protected override string DownloadUrlRoot
|
||||
|
|
|
|||
|
|
@ -28,7 +28,14 @@ public MediaCoverController(IAppFolderInfo appFolderInfo, IDiskProvider diskProv
|
|||
[HttpGet(@"{movieId:int}/{filename:regex((.+)\.(jpg|png|gif))}")]
|
||||
public IActionResult GetMediaCover(int movieId, string filename)
|
||||
{
|
||||
var filePath = Path.Combine(_appFolderInfo.GetAppDataPath(), "MediaCover", movieId.ToString(), filename);
|
||||
var mediaCoverPath = Path.GetFullPath(Path.Combine(_appFolderInfo.GetAppDataPath(), "MediaCover"));
|
||||
var filePath = Path.GetFullPath(Path.Combine(mediaCoverPath, movieId.ToString(), filename));
|
||||
|
||||
// Prevent path traversal - ensure path stays within MediaCover folder
|
||||
if (!filePath.StartsWith(mediaCoverPath + Path.DirectorySeparatorChar))
|
||||
{
|
||||
return NotFound();
|
||||
}
|
||||
|
||||
if (!_diskProvider.FileExists(filePath) || _diskProvider.GetFileSize(filePath) == 0)
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue