Skip to content

Commit b5eb4d6

Browse files
KSemenenkoKSemenenko
committed
sanitiation
1 parent 7c29841 commit b5eb4d6

File tree

13 files changed

+439
-18
lines changed

13 files changed

+439
-18
lines changed

‎Integraions/ManagedCode.Storage.Server/ChunkUpload/ChunkUploadService.cs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ public async Task<Result> AppendChunkAsync(FileUploadPayload payload, Cancellati
4747
var descriptor = payload.Payload;
4848
var uploadId = ChunkUploadDescriptor.ResolveUploadId(descriptor);
4949

50-
var session = _sessions.GetOrAdd(uploadId, static (key, state) =>
50+
// Sanitize upload ID to prevent path traversal
51+
var sanitizedUploadId = SanitizeUploadId(uploadId);
52+
53+
var session = _sessions.GetOrAdd(sanitizedUploadId, static (key, state) =>
5154
{
5255
var descriptor = state.Payload;
5356
var workingDirectory = Path.Combine(state.Options.TempPath, key);
@@ -151,6 +154,29 @@ public void Abort(string uploadId)
151154
}
152155
}
153156

157+
private static string SanitizeUploadId(string uploadId)
158+
{
159+
if (string.IsNullOrWhiteSpace(uploadId))
160+
throw new ArgumentException("Upload ID cannot be null or empty", nameof(uploadId));
161+
162+
// Remove any path traversal sequences
163+
uploadId = uploadId.Replace("..", string.Empty, StringComparison.Ordinal);
164+
uploadId = uploadId.Replace("/", string.Empty, StringComparison.Ordinal);
165+
uploadId = uploadId.Replace("\\", string.Empty, StringComparison.Ordinal);
166+
167+
// Only allow alphanumeric characters, hyphens, and underscores
168+
var allowedChars = uploadId.Where(c => char.IsLetterOrDigit(c) || c == '-' || c == '_').ToArray();
169+
var sanitized = new string(allowedChars);
170+
171+
if (string.IsNullOrWhiteSpace(sanitized))
172+
throw new ArgumentException("Upload ID contains only invalid characters", nameof(uploadId));
173+
174+
if (sanitized.Length > 128)
175+
throw new ArgumentException("Upload ID is too long", nameof(uploadId));
176+
177+
return sanitized;
178+
}
179+
154180
private static async Task MergeChunksAsync(string destinationFile, IReadOnlyCollection<string> chunkFiles, CancellationToken cancellationToken)
155181
{
156182
await using var destination = new FileStream(destinationFile, FileMode.Create, FileAccess.Write, FileShare.None, bufferSize: MergeBufferSize, useAsync: true);

‎Integraions/ManagedCode.Storage.Server/Controllers/StorageControllerBase.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ public virtual async Task<Result<BlobMetadata>> UploadAsync([FromForm] IFormFile
5757
return Result<BlobMetadata>.Fail(HttpStatusCode.BadRequest, "File payload is missing");
5858
}
5959

60+
// Validate file size if enabled
61+
if (_options.EnableFileSizeValidation && _options.MaxFileSize > 0 && file.Length > _options.MaxFileSize)
62+
{
63+
return Result<BlobMetadata>.Fail(HttpStatusCode.RequestEntityTooLarge,
64+
$"File size {file.Length} bytes exceeds maximum allowed size of {_options.MaxFileSize} bytes");
65+
}
66+
6067
try
6168
{
6269
return await Result.From(() => this.UploadFormFileAsync(Storage, file, cancellationToken: cancellationToken), cancellationToken);
@@ -164,6 +171,13 @@ public virtual async Task<Result> UploadChunkAsync([FromForm] FileUploadPayload
164171
return Result.Fail(HttpStatusCode.BadRequest, "UploadId is required");
165172
}
166173

174+
// Validate chunk size if enabled
175+
if (_options.EnableFileSizeValidation && _options.MaxChunkSize > 0 && payload.File.Length > _options.MaxChunkSize)
176+
{
177+
return Result.Fail(HttpStatusCode.RequestEntityTooLarge,
178+
$"Chunk size {payload.File.Length} bytes exceeds maximum allowed chunk size of {_options.MaxChunkSize} bytes");
179+
}
180+
167181
return await ChunkUploadService.AppendChunkAsync(payload, cancellationToken);
168182
}
169183

@@ -228,6 +242,16 @@ public class StorageServerOptions
228242
/// </summary>
229243
public const int DefaultMultipartBoundaryLengthLimit = 70;
230244

245+
/// <summary>
246+
/// Default maximum file size: 100 MB.
247+
/// </summary>
248+
public const long DefaultMaxFileSize = 100 * 1024 * 1024;
249+
250+
/// <summary>
251+
/// Default maximum chunk size: 10 MB.
252+
/// </summary>
253+
public const long DefaultMaxChunkSize = 10 * 1024 * 1024;
254+
231255
/// <summary>
232256
/// Gets or sets a value indicating whether range processing is enabled for streaming responses.
233257
/// </summary>
@@ -242,4 +266,22 @@ public class StorageServerOptions
242266
/// Gets or sets the maximum allowed length for multipart boundaries when parsing raw upload streams.
243267
/// </summary>
244268
public int MultipartBoundaryLengthLimit { get; set; } = DefaultMultipartBoundaryLengthLimit;
269+
270+
/// <summary>
271+
/// Gets or sets the maximum file size in bytes that can be uploaded. Set to 0 to disable the limit.
272+
/// Default is 100 MB.
273+
/// </summary>
274+
public long MaxFileSize { get; set; } = DefaultMaxFileSize;
275+
276+
/// <summary>
277+
/// Gets or sets the maximum chunk size in bytes for chunk uploads. Set to 0 to disable the limit.
278+
/// Default is 10 MB.
279+
/// </summary>
280+
public long MaxChunkSize { get; set; } = DefaultMaxChunkSize;
281+
282+
/// <summary>
283+
/// Gets or sets whether file size validation is enabled.
284+
/// Default is true.
285+
/// </summary>
286+
public bool EnableFileSizeValidation { get; set; } = true;
245287
}

‎ManagedCode.Storage.VirtualFileSystem/Core/VfsPath.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.IO;
4+
using System.Linq;
45

56
namespace ManagedCode.Storage.VirtualFileSystem.Core;
67

@@ -95,6 +96,14 @@ public string ToBlobKey()
9596
/// </summary>
9697
private static string NormalizePath(string path)
9798
{
99+
// Security: Check for null bytes (potential security issue)
100+
if (path.Contains('\0'))
101+
throw new ArgumentException("Path contains null bytes", nameof(path));
102+
103+
// Security: Check for control characters
104+
if (path.Any(c => char.IsControl(c) && c != '\t' && c != '\r' && c != '\n'))
105+
throw new ArgumentException("Path contains control characters", nameof(path));
106+
98107
// 1. Replace backslashes with forward slashes
99108
path = path.Replace('\\', '/');
100109

‎Storages/ManagedCode.Storage.FileSystem/FileSystemStorage.cs‎

Lines changed: 173 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,50 @@ public override async IAsyncEnumerable<BlobMetadata> GetBlobMetadataListAsync(st
5555
if (cancellationToken.IsCancellationRequested)
5656
yield break;
5757

58-
var blobMetadata = await GetBlobMetadataAsync(file, cancellationToken);
58+
var relativePath = Path.GetRelativePath(StorageClient, file)
59+
.Replace('\\', '/');
60+
61+
var (relativeDirectory, relativeFileName) = SplitRelativePath(relativePath);
62+
MetadataOptions options = new()
63+
{
64+
FileName = relativeFileName,
65+
Directory = relativeDirectory
66+
};
67+
68+
var blobMetadata = await GetBlobMetadataAsync(options, cancellationToken);
5969
cancellationToken.ThrowIfCancellationRequested();
6070

6171
if (blobMetadata.IsSuccess)
72+
{
6273
yield return blobMetadata.Value;
74+
continue;
75+
}
6376
}
6477
}
6578

79+
private static (string? Directory, string FileName) SplitRelativePath(string relativePath)
80+
{
81+
if (string.IsNullOrWhiteSpace(relativePath))
82+
throw new ArgumentException("Relative path cannot be null or empty", nameof(relativePath));
83+
84+
var normalizedPath = relativePath.Replace('\\', '/');
85+
var separatorIndex = normalizedPath.LastIndexOf('/');
86+
87+
if (separatorIndex < 0)
88+
return (null, normalizedPath);
89+
90+
var directory = separatorIndex == 0
91+
? null
92+
: normalizedPath[..separatorIndex];
93+
94+
var fileName = normalizedPath[(separatorIndex + 1)..];
95+
96+
if (string.IsNullOrWhiteSpace(fileName))
97+
throw new InvalidOperationException($"Invalid relative path: '{relativePath}'");
98+
99+
return (string.IsNullOrWhiteSpace(directory) ? null : directory, fileName);
100+
}
101+
66102
public override async Task<Result<Stream>> GetStreamAsync(string fileName, CancellationToken cancellationToken = default)
67103
{
68104
try
@@ -315,19 +351,147 @@ protected override async Task<Result<bool>> HasLegalHoldInternalAsync(LegalHoldO
315351

316352
private string GetPathFromOptions(BaseOptions options)
317353
{
318-
string filePath;
319-
if (options.Directory is not null)
354+
if (string.IsNullOrWhiteSpace(options.FileName))
355+
throw new ArgumentException("File name cannot be null or empty", nameof(options.FileName));
356+
357+
var (directoryFromFileName, fileNameOnly) = SplitDirectoryFromFileName(options.FileName);
358+
359+
// Sanitize and validate components
360+
var sanitizedFileName = SanitizeFileName(fileNameOnly);
361+
362+
var combinedDirectory = CombineDirectoryParts(options.Directory, directoryFromFileName);
363+
var sanitizedDirectory = combinedDirectory is not null
364+
? SanitizeDirectory(combinedDirectory)
365+
: null;
366+
367+
if (sanitizedDirectory is not null)
320368
{
321-
EnsureDirectoryExist(options.Directory);
322-
filePath = Path.Combine(StorageClient, options.Directory, options.FileName);
369+
EnsureDirectoryExist(sanitizedDirectory);
323370
}
324-
else
371+
372+
string filePath = sanitizedDirectory is not null
373+
? Path.Combine(StorageClient, sanitizedDirectory, sanitizedFileName)
374+
: Path.Combine(StorageClient, sanitizedFileName);
375+
376+
// Get full paths for comparison
377+
var fullPath = Path.GetFullPath(filePath);
378+
var baseFullPath = Path.GetFullPath(StorageClient);
379+
380+
// Verify the final path is within StorageClient directory
381+
if (!fullPath.StartsWith(baseFullPath, StringComparison.OrdinalIgnoreCase))
325382
{
326-
filePath = Path.Combine(StorageClient, options.FileName);
383+
throw new UnauthorizedAccessException($"Access to path '{options.FileName}' is denied. Path traversal detected.");
384+
}
385+
386+
EnsureDirectoryExist(Path.GetDirectoryName(fullPath)!);
387+
return fullPath;
388+
}
389+
390+
private static (string? Directory, string FileName) SplitDirectoryFromFileName(string fileName)
391+
{
392+
var normalized = fileName.Replace('\\', '/');
393+
var lastSlash = normalized.LastIndexOf('/');
394+
395+
if (lastSlash < 0)
396+
return (null, normalized);
397+
398+
var directory = normalized[..lastSlash];
399+
var name = normalized[(lastSlash + 1)..];
400+
return (directory, name);
401+
}
402+
403+
private static string? CombineDirectoryParts(string? primary, string? secondary)
404+
{
405+
var parts = new List<string>();
406+
407+
void AddPart(string? value)
408+
{
409+
if (string.IsNullOrWhiteSpace(value))
410+
return;
411+
412+
var normalized = value.Replace('\\', '/');
413+
foreach (var segment in normalized.Split('/', StringSplitOptions.RemoveEmptyEntries))
414+
{
415+
parts.Add(segment);
416+
}
417+
}
418+
419+
AddPart(primary);
420+
AddPart(secondary);
421+
422+
if (parts.Count == 0)
423+
return null;
424+
425+
return string.Join('/', parts);
426+
}
427+
428+
private static string SanitizeFileName(string fileName)
429+
{
430+
if (string.IsNullOrWhiteSpace(fileName))
431+
throw new ArgumentException("File name cannot be null or empty", nameof(fileName));
432+
433+
var originalFileName = fileName;
434+
435+
// Check for path traversal attempts - throw exception if detected
436+
if (fileName.Contains("..", StringComparison.Ordinal))
437+
throw new UnauthorizedAccessException($"Access to path '{originalFileName}' is denied. Path traversal detected.");
438+
439+
// If there are path separators, extract only the filename part
440+
// This handles cases like /tmp/file.txt -> file.txt
441+
if (fileName.Contains('/') || fileName.Contains('\\'))
442+
{
443+
fileName = Path.GetFileName(fileName);
444+
}
445+
446+
if (string.IsNullOrWhiteSpace(fileName))
447+
throw new ArgumentException("Invalid file name", nameof(fileName));
448+
449+
// Remove any invalid filename characters
450+
var invalidChars = Path.GetInvalidFileNameChars();
451+
var sanitized = fileName;
452+
foreach (var c in invalidChars)
453+
{
454+
sanitized = sanitized.Replace(c.ToString(), string.Empty);
455+
}
456+
457+
if (string.IsNullOrWhiteSpace(sanitized))
458+
throw new ArgumentException("File name contains only invalid characters", nameof(fileName));
459+
460+
return sanitized;
461+
}
462+
463+
private static string SanitizeDirectory(string directory)
464+
{
465+
if (string.IsNullOrWhiteSpace(directory))
466+
return string.Empty;
467+
468+
var originalDirectory = directory;
469+
470+
// Check for path traversal attempts - throw exception if detected
471+
if (directory.Contains("..", StringComparison.Ordinal))
472+
throw new UnauthorizedAccessException($"Access to path '{originalDirectory}' is denied. Path traversal detected.");
473+
474+
// Normalize path separators
475+
directory = directory.Replace('\\', '/');
476+
477+
// Remove leading and trailing slashes
478+
directory = directory.Trim('/');
479+
480+
// Validate each directory segment
481+
var segments = directory.Split('/', StringSplitOptions.RemoveEmptyEntries);
482+
var invalidChars = Path.GetInvalidFileNameChars();
483+
484+
foreach (var segment in segments)
485+
{
486+
if (segment.IndexOfAny(invalidChars) >= 0)
487+
throw new ArgumentException($"Directory path contains invalid characters: {segment}", nameof(directory));
488+
489+
// Additional check for suspicious segments
490+
if (segment == ".." || segment == ".")
491+
throw new UnauthorizedAccessException($"Access to path '{originalDirectory}' is denied. Path traversal detected.");
327492
}
328493

329-
EnsureDirectoryExist(Path.GetDirectoryName(filePath)!);
330-
return filePath;
494+
return string.Join(Path.DirectorySeparatorChar, segments);
331495
}
332496

333497
private void EnsureDirectoryExist(string directory)

‎Tests/ManagedCode.Storage.Tests/AspNetTests/Abstracts/BaseStreamControllerTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public async Task StreamFile_WhenFileExists_SaveToTempStorage_ReturnSuccess()
4646
var streamedValue = streamFileResult.Value ?? throw new InvalidOperationException("Stream result does not contain a stream");
4747

4848
await using var stream = streamedValue;
49-
await using var newLocalFile = await LocalFile.FromStreamAsync(stream, Path.GetTempPath(), Guid.NewGuid()
49+
await using var newLocalFile = await LocalFile.FromStreamAsync(stream, Environment.CurrentDirectory, Guid.NewGuid()
5050
.ToString("N") + extension);
5151

5252
var streamedFileCRC = Crc32Helper.CalculateFileCrc(newLocalFile.FilePath);

‎Tests/ManagedCode.Storage.Tests/AspNetTests/Azure/AzureSignalRStorageTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public async Task DownloadStreamAsync_WhenBlobExists_ShouldDownloadContent()
120120

121121
var expectedCrc = Crc32Helper.CalculateFileCrc(localFile.FilePath);
122122
memory.Position = 0;
123-
await using var downloadedFile = await LocalFile.FromStreamAsync(memory, Path.GetTempPath(), Guid.NewGuid().ToString("N") + localFile.FileInfo.Extension);
123+
await using var downloadedFile = await LocalFile.FromStreamAsync(memory, Environment.CurrentDirectory, Guid.NewGuid().ToString("N") + localFile.FileInfo.Extension);
124124
var downloadedCrc = Crc32Helper.CalculateFileCrc(downloadedFile.FilePath);
125125
downloadedCrc.ShouldBe(expectedCrc);
126126

‎Tests/ManagedCode.Storage.Tests/Common/FileHelper.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public static LocalFile GenerateLocalFile(LocalFile localFile, int byteSize)
2424

2525
public static LocalFile GenerateLocalFile(string fileName, int byteSize)
2626
{
27-
var path = Path.Combine(Path.GetTempPath(), fileName);
27+
var path = Path.Combine(Environment.CurrentDirectory, fileName);
2828
var localFile = new LocalFile(path);
2929

3030
var fs = localFile.FileStream;
@@ -94,4 +94,4 @@ public static string GenerateRandomFileContent(int charCount = 250_000)
9494
.Select(s => s[Random.Next(s.Length)])
9595
.ToArray());
9696
}
97-
}
97+
}

‎Tests/ManagedCode.Storage.Tests/Core/Crc32HelperTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public class Crc32HelperTests
1212
[Fact]
1313
public void CalculateFileCrc_ShouldMatchInMemoryCalculation()
1414
{
15-
var tempPath = Path.Combine(Path.GetTempPath(), $"crc-test-{Guid.NewGuid():N}.bin");
15+
var tempPath = Path.Combine(Environment.CurrentDirectory, $"crc-test-{Guid.NewGuid():N}.bin");
1616
try
1717
{
1818
var payload = new byte[4096 + 123];

‎Tests/ManagedCode.Storage.Tests/Server/ChunkUploadServiceTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace ManagedCode.Storage.Tests.Server;
1515

1616
public class ChunkUploadServiceTests : IAsyncLifetime
1717
{
18-
private readonly string _root = Path.Combine(Path.GetTempPath(), "managedcode-chunk-tests", Guid.NewGuid().ToString());
18+
private readonly string _root = Path.Combine(Environment.CurrentDirectory, "managedcode-chunk-tests", Guid.NewGuid().ToString());
1919
private ChunkUploadOptions _options = null!;
2020

2121
public Task InitializeAsync()

0 commit comments

Comments
 (0)