diff --git a/Engine/Internal/Infrastructure/Endpoints/EndPoint.cs b/Engine/Internal/Infrastructure/Endpoints/EndPoint.cs index d7cea1975..a3106f0ed 100644 --- a/Engine/Internal/Infrastructure/Endpoints/EndPoint.cs +++ b/Engine/Internal/Infrastructure/Endpoints/EndPoint.cs @@ -98,27 +98,39 @@ private async Task Listen() { if (Socket == null) throw new InvalidOperationException("The endpoint has not been started"); - try + while (!_shuttingDown) { - do + try { Handle(await Socket.AcceptAsync()); } - while (!_shuttingDown); - } - catch (Exception e) - { - if (!_shuttingDown && !ConnectionExceptions.IsGracefulDisconnect(e)) + catch (ObjectDisposedException) { - Logger.LogError(e, "Failed to accept incoming connection"); + break; + } + catch (OperationCanceledException) + { + break; + } + catch (Exception e) + { + if (_shuttingDown) + { + break; + } + + if (!ConnectionExceptions.IsGracefulDisconnect(e)) + { + Logger.LogError(e, "Failed to accept incoming connection"); + } + + await Task.Delay(500); } } } private void Handle(Socket client) { - using var _ = ExecutionContext.SuppressFlow(); - Task.Run(() => Accept(client)); } diff --git a/Engine/Ioxide/GenHTTP.Engine.Ioxide.csproj b/Engine/Ioxide/GenHTTP.Engine.Ioxide.csproj index 65c05ad73..aa0549a65 100644 --- a/Engine/Ioxide/GenHTTP.Engine.Ioxide.csproj +++ b/Engine/Ioxide/GenHTTP.Engine.Ioxide.csproj @@ -13,7 +13,9 @@ - + + + diff --git a/Engine/Ioxide/Protocol/Drivers/Tcp/Http1Driver.cs b/Engine/Ioxide/Protocol/Drivers/Tcp/Http1Driver.cs index cae1f623c..ca72d0142 100644 --- a/Engine/Ioxide/Protocol/Drivers/Tcp/Http1Driver.cs +++ b/Engine/Ioxide/Protocol/Drivers/Tcp/Http1Driver.cs @@ -7,6 +7,7 @@ using GenHTTP.Engine.Shared.Types; +using Glyph11; using Glyph11.Parser; using Glyph11.Parser.UltraHardened; using Glyph11.Pico; @@ -18,6 +19,8 @@ using IoConnection = ioxide.TcpConnection; using GenHTTP.Engine.Ioxide.Protocol.Responses; +using StringContent = GenHTTP.Modules.IO.Strings.StringContent; + namespace GenHTTP.Engine.Ioxide.Protocol.Drivers.Tcp; /// Serves HTTP/1.1 on one connection, request after request. @@ -105,6 +108,10 @@ internal static async Task RunAsync(IServer server, IEndPoint endPoint, IDuplexP } } } + catch (HttpParseException pe) + { + await SendErrorAsync(server, writer, pe, (ResponseStatus)pe.StatusCode); + } finally { WarnIfThreadHopped(server, reactorThreadId, "before-return"); @@ -115,6 +122,28 @@ internal static async Task RunAsync(IServer server, IEndPoint endPoint, IDuplexP } } + private static async ValueTask SendErrorAsync(IServer server, PipeWriter writer, Exception e, ResponseStatus status) + { + try + { + var message = server.Development ? e.ToString() : e.Message; + + var response = new ResponseBuilder() + .Status(status) + .Connection(Connection.Close) + .Content(new StringContent(message)) + .Build(); + + await Http1Responder.WriteAsync(writer, null, response, keepAlive: false, headRequest: false); + + await writer.FlushAsync(); + } + catch + { + /* no recovery here */ + } + } + // Parses one request head, through whichever parser this process was started with. private static bool TryParseRequest(ref ReadOnlySequence buffer, BinaryRequest into) => UsePico ? TryParseRequestPico(ref buffer, into) : TryParseRequestGlyph11(ref buffer, into); diff --git a/Engine/Shared/Types/Body/ChunkedBodyStream.cs b/Engine/Shared/Types/Body/ChunkedBodyStream.cs index dc9d6f99f..324f41a71 100644 --- a/Engine/Shared/Types/Body/ChunkedBodyStream.cs +++ b/Engine/Shared/Types/Body/ChunkedBodyStream.cs @@ -98,7 +98,7 @@ public override async ValueTask ReadAsync(Memory buffer, Cancellation if (result.IsCompleted) { - throw new InvalidDataException("Unexpected end of chunked body"); + throw new ProviderException(ResponseStatus.BadRequest, "Unexpected end of chunked body"); } continue; diff --git a/Engine/Shared/Types/Body/MemoryConsumptionStrategy.cs b/Engine/Shared/Types/Body/MemoryConsumptionStrategy.cs index 45c526fc5..0d2a2b219 100644 --- a/Engine/Shared/Types/Body/MemoryConsumptionStrategy.cs +++ b/Engine/Shared/Types/Body/MemoryConsumptionStrategy.cs @@ -1,5 +1,7 @@ using System.Buffers; using System.IO.Pipelines; +using GenHTTP.Api.Content; +using GenHTTP.Api.Protocol; namespace GenHTTP.Engine.Shared.Types.Body; @@ -41,7 +43,7 @@ public ValueTask> ObtainAsync() { return new(_memory.Value); } - + if (_length is not null) { return ReadLength(); @@ -55,7 +57,7 @@ private async ValueTask> ReadLength() var reader = Reader; var length = _length!.Value; - + while (true) { var result = await reader.ReadAsync(); @@ -63,6 +65,12 @@ private async ValueTask> ReadLength() if (result.Buffer.Length < length) { reader.AdvanceTo(result.Buffer.Start, result.Buffer.End); + + if (result.IsCompleted) + { + throw new ProviderException(ResponseStatus.BadRequest, "Unexpected end of body"); + } + continue; } @@ -83,15 +91,15 @@ private async ValueTask> ReadLength() return (_memory = linearized).Value; } } - + private async ValueTask> ReadChunked() { var chunkedStream = new ChunkedBodyStream(Reader); - + var writer = new ArrayBufferWriter(); var pool = ArrayPool.Shared; - + var buffer = pool.Rent(16 * 1024); try @@ -115,10 +123,10 @@ private async ValueTask> ReadChunked() } await chunkedStream.DrainAsync(); - + return (_memory = writer.WrittenMemory).Value; } - + public ValueTask DrainAsync() { if (_readResult is not null) diff --git a/Modules/DirectoryBrowsing/Provider/ListingRouter.cs b/Modules/DirectoryBrowsing/Provider/ListingRouter.cs index 7539fbb9b..fde9c8f87 100644 --- a/Modules/DirectoryBrowsing/Provider/ListingRouter.cs +++ b/Modules/DirectoryBrowsing/Provider/ListingRouter.cs @@ -29,7 +29,11 @@ public ListingRouter(IResourceTree tree) public async ValueTask HandleAsync(IRequest request) { - var (node, resource) = await Tree.FindAsync(request.Header.Target); + var target = request.Header.Target; + + target.DenyPathTraversal(); + + var (node, resource) = await Tree.FindAsync(target); if (resource is not null) { diff --git a/Modules/Files/Multi/AbstractAssetsHandler.cs b/Modules/Files/Multi/AbstractAssetsHandler.cs index 5ae392ccc..7c2d61fe1 100644 --- a/Modules/Files/Multi/AbstractAssetsHandler.cs +++ b/Modules/Files/Multi/AbstractAssetsHandler.cs @@ -22,12 +22,14 @@ protected AbstractAssetsHandler(List algorithms, char sep public async ValueTask HandleAsync(IRequest request) { var target = request.Header.Target; - + if (target.HasTrailingSlash) { return null; } + target.DenyPathTraversal(); + if (_preCompression.Enabled) { var handled = await TryGetPreCompressed(request); diff --git a/Modules/Files/Multi/Ioxide/IoxideFilesHandler.cs b/Modules/Files/Multi/Ioxide/IoxideFilesHandler.cs index b742d7188..ef2ada598 100644 --- a/Modules/Files/Multi/Ioxide/IoxideFilesHandler.cs +++ b/Modules/Files/Multi/Ioxide/IoxideFilesHandler.cs @@ -60,6 +60,8 @@ public ValueTask PrepareAsync(IServer server) var target = request.Header.Target; + target.DenyPathTraversal(); + if (target.HasTrailingSlash) { return default; // a directory request, not a file (no directory index) diff --git a/Modules/IO/Extensions.Request.Security.cs b/Modules/IO/Extensions.Request.Security.cs new file mode 100644 index 000000000..cf65f4df6 --- /dev/null +++ b/Modules/IO/Extensions.Request.Security.cs @@ -0,0 +1,58 @@ +using System.Runtime.CompilerServices; + +using GenHTTP.Api.Content; +using GenHTTP.Api.Protocol; + +namespace GenHTTP.Modules.IO; + +public static class RequestSecurityExtensions +{ + + /// + /// Checks all remaining, non-routed segments of the request target for + /// path traversal attacks and throws a provider exception if one + /// is detected. + /// + /// The request target to be checked + public static void DenyPathTraversal(this IRequestTarget target) + { + var index = 0; + + PathSegment? segment; + + while ((segment = target.Next(index++)) != null) + { + if (IsDotSegment(segment.Value.Bytes.Span)) + { + throw new ProviderException(ResponseStatus.BadRequest, "Potential path traversal detected"); + } + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsDotSegment(ReadOnlySpan segment) + { + var dots = 0; + + for (var i = 0; i < segment.Length;) + { + if (segment[i] == (byte)'.') + { + dots++; + i++; + } + else if (segment[i] == (byte)'%' && i + 2 < segment.Length && segment[i + 1] == (byte)'2' && (segment[i + 2] == (byte)'e' || segment[i + 2] == (byte)'E')) + { + dots++; + i += 3; + } + else + { + return false; + } + } + + return dots is 1 or 2; + } + +} diff --git a/Modules/IO/Ranges/RangedStream.cs b/Modules/IO/Ranges/RangedStream.cs index 0d988d586..4b93dc17b 100644 --- a/Modules/IO/Ranges/RangedStream.cs +++ b/Modules/IO/Ranges/RangedStream.cs @@ -52,31 +52,19 @@ public RangedStream(Stream target, ulong start, ulong end) public override void Write(byte[] buffer, int offset, int count) { - if (Position > End) + if (Position > End || (Position + count) <= Start) { + Position += count; return; } - long actualOffset = offset; - long actualCount = count; + var writeStart = Math.Max(Position, Start); + var writeEnd = Math.Min(Position + count - 1, End); - if (Position < Start) - { - actualOffset += (int)(Start - Position); - actualCount -= (int)(Start - Position); - } - - if ((Start + actualCount) > (End + 1)) - { - actualCount = Math.Min(End - Start + 1, actualCount); - } - - if (actualOffset < buffer.Length) - { - var toWrite = Math.Min(buffer.Length - actualOffset, actualCount); + var actualOffset = offset + (writeStart - Position); + var actualCount = writeEnd - writeStart + 1; - Target.Write(buffer, (int)actualOffset, (int)toWrite); - } + Target.Write(buffer, (int)actualOffset, (int)actualCount); Position += count; } diff --git a/Modules/SinglePageApplications/Provider/SinglePageProvider.cs b/Modules/SinglePageApplications/Provider/SinglePageProvider.cs index 7c7310035..0aca61cbe 100644 --- a/Modules/SinglePageApplications/Provider/SinglePageProvider.cs +++ b/Modules/SinglePageApplications/Provider/SinglePageProvider.cs @@ -44,7 +44,9 @@ public SinglePageProvider(IResourceTree tree, bool serverSideRouting) public async ValueTask HandleAsync(IRequest request) { - if (request.Header.Target.Current == null) + var target = request.Header.Target; + + if (target.Current == null) { var index = await GetIndex(); @@ -55,6 +57,8 @@ public SinglePageProvider(IResourceTree tree, bool serverSideRouting) } else { + target.DenyPathTraversal(); + var result = await Resources.HandleAsync(request); if (result == null) diff --git a/Modules/StaticWebsites/Provider/StaticWebsiteHandler.cs b/Modules/StaticWebsites/Provider/StaticWebsiteHandler.cs index ab2f55445..998ce28a3 100644 --- a/Modules/StaticWebsites/Provider/StaticWebsiteHandler.cs +++ b/Modules/StaticWebsites/Provider/StaticWebsiteHandler.cs @@ -38,6 +38,8 @@ public StaticWebsiteHandler(IResourceTree tree) public async ValueTask HandleAsync(IRequest request) { var target = request.Header.Target; + + target.DenyPathTraversal(); if (target.HasTrailingSlash) { diff --git a/Testing/Acceptance/Engine/Body/ChunkedBodyStreamTests.cs b/Testing/Acceptance/Engine/Body/ChunkedBodyStreamTests.cs index 5384a79fc..8c442d756 100644 --- a/Testing/Acceptance/Engine/Body/ChunkedBodyStreamTests.cs +++ b/Testing/Acceptance/Engine/Body/ChunkedBodyStreamTests.cs @@ -1,6 +1,6 @@ using System.IO.Pipelines; using System.Text; - +using GenHTTP.Api.Content; using GenHTTP.Engine.Shared.Types.Body; namespace GenHTTP.Testing.Acceptance.Engine.Body; @@ -87,7 +87,7 @@ public async Task TestTruncatedChunkThrows() var stream = await CreateAsync("5\r\nHel"); #pragma warning disable CA2022 // expected to throw before any byte count is returned - await Assert.ThrowsExactlyAsync(async () => await stream.ReadAsync(new byte[16])); + await Assert.ThrowsExactlyAsync(async () => await stream.ReadAsync(new byte[16])); #pragma warning restore CA2022 } diff --git a/Testing/Acceptance/Engine/Body/DrainBodyTests.cs b/Testing/Acceptance/Engine/Body/DrainBodyTests.cs index 075bf0ede..e65a6c289 100644 --- a/Testing/Acceptance/Engine/Body/DrainBodyTests.cs +++ b/Testing/Acceptance/Engine/Body/DrainBodyTests.cs @@ -2,7 +2,6 @@ using GenHTTP.Api.Content; using GenHTTP.Api.Infrastructure; using GenHTTP.Api.Protocol; -using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine; namespace GenHTTP.Testing.Acceptance.Engine.Body; diff --git a/Testing/Acceptance/Engine/Body/MemoryBodyTests.cs b/Testing/Acceptance/Engine/Body/MemoryBodyTests.cs index 3e6a8eb7e..83a4afe40 100644 --- a/Testing/Acceptance/Engine/Body/MemoryBodyTests.cs +++ b/Testing/Acceptance/Engine/Body/MemoryBodyTests.cs @@ -1,4 +1,6 @@ using System.Net; +using System.Net.Sockets; +using System.Text; using GenHTTP.Api.Content; using GenHTTP.Api.Infrastructure; using GenHTTP.Api.Protocol; @@ -173,6 +175,41 @@ public async Task TestKeepAliveChunked(ServerEngine engine) CollectionAssert.AreEqual(second, await resp2.Content.ReadAsByteArrayAsync()); } + [TestMethod] + public async Task TestTruncatedContentLengthDoesNotSpin() + { + await using var runner = await TestHost.RunAsync(new BodyEchoHandler().Wrap()); + + using var client = new TcpClient("127.0.0.1", runner.Port) + { + ReceiveTimeout = 15000 + }; + + var stream = client.GetStream(); + + var request = "POST / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: 100\r\n" + + "\r\n" + + "short"; // only 5 of the promised 100 bytes + + var bytes = Encoding.ASCII.GetBytes(request); + + await stream.WriteAsync(bytes); + await stream.FlushAsync(); + + // signal end of the request body without ever sending the rest + client.Client.Shutdown(SocketShutdown.Send); + + using var reader = new StreamReader(stream, leaveOpen: true); + + var response = await reader.ReadToEndAsync(); + + // the connection is closed cleanly with an error instead of spinning; + // an empty response would mean the server never got past the body read + Assert.IsTrue(response.StartsWith("HTTP/1.1 400", StringComparison.Ordinal), response); + } + #endregion #region Supporting types diff --git a/Testing/Acceptance/Modules/IO/PathTraversalTests.cs b/Testing/Acceptance/Modules/IO/PathTraversalTests.cs new file mode 100644 index 000000000..cb3c13b84 --- /dev/null +++ b/Testing/Acceptance/Modules/IO/PathTraversalTests.cs @@ -0,0 +1,157 @@ +using System.Net; + +using GenHTTP.Api.Infrastructure; + +using GenHTTP.Modules.DirectoryBrowsing; +using GenHTTP.Modules.Files; +using GenHTTP.Modules.IO; +using GenHTTP.Modules.SinglePageApplications; +using GenHTTP.Modules.StaticWebsites; + +namespace GenHTTP.Testing.Acceptance.Modules.IO; + +/// +/// Verifies that every file serving handler rejects path traversal attempts +/// via . +/// +/// +/// Each handler is probed with a different forged segment so that, taken +/// together, the tests exercise all branches of the detection logic (plain +/// dots, upper- and lower-case percent encoding, mixed forms and the negative +/// cases that must not be denied). +/// +[TestClass] +public sealed class PathTraversalTests +{ + + #region Denial (one case per affected handler) + + [TestMethod] + [MultiEngineTest] + public async Task TestAssetsTreeDeniesEncodedTraversal(ServerEngine engine) + { + // TreeAssetsHandler (via AbstractAssetsHandler) - percent encoded ".." ("%2e%2e") + await using var runner = await TestHost.RunAsync(Assets.From(ResourceTree.FromDirectory(CreateRoot())), engine: engine); + + using var response = await GetForgedAsync(runner, "/sub/%2e%2e/file.txt"); + + await response.AssertStatusAsync(HttpStatusCode.BadRequest); + } + + [TestMethod] + [MultiEngineTest] + public async Task TestAssetsDirectoryDeniesLiteralTraversal(ServerEngine engine) + { + // BuiltInFileAssetHandler (via AbstractAssetsHandler) - literal ".." + await using var runner = await TestHost.RunAsync(Assets.From(CreateRoot()), engine: engine); + + using var response = await GetForgedAsync(runner, "/sub/../file.txt"); + + await response.AssertStatusAsync(HttpStatusCode.BadRequest); + } + + [TestMethod] + [MultiEngineTest] + public async Task TestStaticWebsiteDeniesUppercaseEncodedTraversal(ServerEngine engine) + { + // StaticWebsiteHandler trailing slash branch (bypasses AbstractAssetsHandler) - upper case "%2E" + await using var runner = await TestHost.RunAsync(StaticWebsite.From(ResourceTree.FromDirectory(CreateRoot())), engine: engine); + + using var response = await GetForgedAsync(runner, "/sub/%2E/"); + + await response.AssertStatusAsync(HttpStatusCode.BadRequest); + } + + [TestMethod] + [MultiEngineTest] + public async Task TestSinglePageDeniesMixedTraversal(ServerEngine engine) + { + // SinglePageProvider - mixed literal and encoded dot (".%2e") + await using var runner = await TestHost.RunAsync(SinglePageApplication.From(ResourceTree.FromDirectory(CreateRoot())), engine: engine); + + using var response = await GetForgedAsync(runner, "/app/.%2e/config.json"); + + await response.AssertStatusAsync(HttpStatusCode.BadRequest); + } + + [TestMethod] + [MultiEngineTest] + public async Task TestDirectoryBrowsingDeniesSingleDot(ServerEngine engine) + { + // ListingRouter (bypasses AbstractAssetsHandler) - single "." + await using var runner = await TestHost.RunAsync(Listing.From(ResourceTree.FromDirectory(CreateRoot())), engine: engine); + + using var response = await GetForgedAsync(runner, "/sub/./"); + + await response.AssertStatusAsync(HttpStatusCode.BadRequest); + } + + #endregion + + #region Non-traversal segments (completes detection coverage) + + [TestMethod] + [MultiEngineTest] + public async Task TestSuspiciousButValidSegmentsAreAllowed(ServerEngine engine) + { + await using var runner = await TestHost.RunAsync(Assets.From(ResourceTree.FromDirectory(CreateRoot())), engine: engine); + + // a genuine file is still served (first byte is not a dot - detection returns immediately) + using var served = await GetForgedAsync(runner, "/file.txt"); + await served.AssertStatusAsync(HttpStatusCode.OK); + + // the following segments look suspicious but are not "." or ".." - the guard must let them + // through (their resolved status is filesystem dependent, so we only assert they are not denied): + + // three dots - the dot count is neither 1 nor 2 + using var tripleDot = await GetForgedAsync(runner, "/.../file.txt"); + Assert.AreNotEqual(HttpStatusCode.BadRequest, tripleDot.StatusCode); + + // percent escape whose first digit is not '2' ("%41" = 'A') + using var notTwo = await GetForgedAsync(runner, "/%41/file.txt"); + Assert.AreNotEqual(HttpStatusCode.BadRequest, notTwo.StatusCode); + + // percent escape "%2x" where x is neither 'e' nor 'E' ("%2b" = '+') + using var notDot = await GetForgedAsync(runner, "/%2b/file.txt"); + Assert.AreNotEqual(HttpStatusCode.BadRequest, notDot.StatusCode); + + // trailing, incomplete percent escape - not enough characters to form "%2e" + using var incomplete = await GetForgedAsync(runner, "/.%/file.txt"); + Assert.AreNotEqual(HttpStatusCode.BadRequest, incomplete.StatusCode); + } + + #endregion + + #region Helpers + + private static string CreateRoot() + { + var root = Directory.CreateTempSubdirectory(); + + File.WriteAllText(Path.Combine(root.FullName, "file.txt"), "This is a file"); + + root.CreateSubdirectory("sub"); + + return root.FullName; + } + + /// + /// Sends a request without client side canonicalization, so that forged + /// segments such as ".." or "%2e%2e" actually reach the server instead of + /// being collapsed by . + /// + private static Task GetForgedAsync(TestHost runner, string rawPath) + { + var uri = new Uri(runner.GetUrl(rawPath), new UriCreationOptions + { + DangerousDisablePathAndQueryCanonicalization = true + }); + + var request = new HttpRequestMessage(HttpMethod.Get, uri); + + return runner.GetResponseAsync(request); + } + + #endregion + +} diff --git a/Testing/Acceptance/Modules/IO/RangedStreamTests.cs b/Testing/Acceptance/Modules/IO/RangedStreamTests.cs index 4b761c3f2..4d113b0a7 100644 --- a/Testing/Acceptance/Modules/IO/RangedStreamTests.cs +++ b/Testing/Acceptance/Modules/IO/RangedStreamTests.cs @@ -49,6 +49,44 @@ public void TestOutOfLargeFile() Assert.AreEqual("", GetRange(0, 10_000, 0, 10, 15_000)); } + [TestMethod] + public void TestEndClampsToPositionNotStart() + { + Assert.AreEqual("01", GetRange(2, 5, 0, 10, 4)); + } + + [TestMethod] + public void TestMultipleBuffersDoNotExceedRange() + { + using var target = new MemoryStream(); + + const int bufferSize = 8; + const ulong start = 6; + const ulong end = 17; // inclusive -> 12 bytes expected + + using var stream = new RangedStream(target, start, end); + + var source = new byte[32]; + for (var i = 0; i < source.Length; i++) + { + source[i] = (byte)i; + } + + for (var offset = 0; offset < source.Length; offset += bufferSize) + { + stream.Write(source, offset, bufferSize); + } + + var written = target.ToArray(); + + Assert.AreEqual((int)(end - start + 1), written.Length); + + for (var i = 0; i < written.Length; i++) + { + Assert.AreEqual((byte)(start + (ulong)i), written[i]); + } + } + [TestMethod] public void TestBasics() {