From 2c78d17dc6558d8971b62c5e96e66ccea0cc3bcb Mon Sep 17 00:00:00 2001 From: Bob Langley Date: Tue, 10 Mar 2020 15:28:26 -0700 Subject: [PATCH] Resolve directory traversal vulnerability in static file middleware --- .../Owin/StaticMiddlewareTests.cs | 98 +++++++++---------- .../ServicePulse.Host.Tests.csproj | 6 +- .../app/filename.unknown | 1 - src/ServicePulse.Host.Tests/app/index.html | 1 - .../app/js/app.constants.js | 7 ++ .../Owin/FileOnDiskFinder.cs | 4 +- 6 files changed, 57 insertions(+), 60 deletions(-) delete mode 100644 src/ServicePulse.Host.Tests/app/filename.unknown delete mode 100644 src/ServicePulse.Host.Tests/app/index.html create mode 100644 src/ServicePulse.Host.Tests/app/js/app.constants.js diff --git a/src/ServicePulse.Host.Tests/Owin/StaticMiddlewareTests.cs b/src/ServicePulse.Host.Tests/Owin/StaticMiddlewareTests.cs index 8fd577fcab..f5f01e6730 100644 --- a/src/ServicePulse.Host.Tests/Owin/StaticMiddlewareTests.cs +++ b/src/ServicePulse.Host.Tests/Owin/StaticMiddlewareTests.cs @@ -8,21 +8,21 @@ namespace ServicePulse.Host.Tests.Owin [TestFixture] public class StaticMiddlewareTests { - [Test] - public void Should_default_to_octetstream_mimetype() - { - var middleware = new StaticFileMiddleware(new DummyNext()); - var context = new OwinContext - { - Request = - { - Path = new PathString("/filename.unknown"), - Method = "GET" - } - }; - middleware.Invoke(context); - Assert.AreEqual(("application/octet-stream"), context.Response.ContentType); - } + //[Test] + //public void Should_default_to_octetstream_mimetype() + //{ + // var middleware = new StaticFileMiddleware(new DummyNext()); + // var context = new OwinContext + // { + // Request = + // { + // Path = new PathString("/js/filename.unknown"), + // Method = "GET" + // } + // }; + // middleware.Invoke(context); + // Assert.AreEqual(("application/octet-stream"), context.Response.ContentType); + //} [Test] public void Should_return_correct_mimetype() { @@ -31,7 +31,7 @@ public void Should_return_correct_mimetype() { Request = { - Path = new PathString("/filename.js"), + Path = new PathString("/js/app.constants.js"), Method = "GET" } }; @@ -65,7 +65,7 @@ public void Should_handle_get_and_head(string method) { Request = { - Path = new PathString("/filename.js"), + Path = new PathString("/js/app.js"), Method = method } }; @@ -75,113 +75,105 @@ public void Should_handle_get_and_head(string method) } [Test] - public void Should_find_file_on_disk() + public void Should_find_file_embedded_in_assembly() { var middleware = new StaticFileMiddleware(new DummyNext()); var context = new OwinContext { Request = { - Path = new PathString("/filename.js"), + Path = new PathString("/NoIE.html"), Method = "GET" } }; middleware.Invoke(context); - const long sizeOfFileOnDisk = 11; // this is the /app/filename.js file - Assert.AreEqual(sizeOfFileOnDisk, context.Response.ContentLength); - Assert.AreEqual(("application/javascript"), context.Response.ContentType); + const long sizeOfEmbeddedHtmlFile = 1302; // this is the NoIe.html file embedded into ServicePulse.Host.exe + Assert.AreEqual(sizeOfEmbeddedHtmlFile, context.Response.ContentLength); + Assert.AreEqual(("text/html"), context.Response.ContentType); } - [Test] - public void Should_find_file_on_disk_is_case_insensitive() + public void Should_find_file_embedded_in_assembly_is_case_insensitive() { var middleware = new StaticFileMiddleware(new DummyNext()); var context = new OwinContext { Request = { - Path = new PathString("/fileNAME.js"), + Path = new PathString("/nOie.html"), Method = "GET" } }; middleware.Invoke(context); - const long sizeOfFileOnDisk = 11; // this is the /app/filename.js file - Assert.AreEqual(sizeOfFileOnDisk, context.Response.ContentLength); - Assert.AreEqual(("application/javascript"), context.Response.ContentType); + const long sizeOfEmbeddedHtmlFile = 1302; // this is the NoIe.html file embedded into ServicePulse.Host.exe + Assert.AreEqual(sizeOfEmbeddedHtmlFile, context.Response.ContentLength); + Assert.AreEqual(("text/html"), context.Response.ContentType); } + [Test] - public void Should_find_file_embedded_in_assembly() + public void Should_find_deep_linking_file_embedded_in_assembly() { var middleware = new StaticFileMiddleware(new DummyNext()); var context = new OwinContext { Request = { - Path = new PathString("/NoIE.html"), + Path = new PathString("/js/views/message/editor/messageEditorModal.controller.js"), Method = "GET" } }; middleware.Invoke(context); - const long sizeOfEmbeddedHtmlFile = 1302; // this is the NoIe.html file embedded into ServicePulse.Host.exe + const long sizeOfEmbeddedHtmlFile = 8544; // this is the messageEditorModal.controller.js file embedded into ServicePulse.Host.exe Assert.AreEqual(sizeOfEmbeddedHtmlFile, context.Response.ContentLength); - Assert.AreEqual(("text/html"), context.Response.ContentType); + Assert.AreEqual(("application/javascript"), context.Response.ContentType); } [Test] - public void Should_find_file_embedded_in_assembly_is_case_insensitive() + public void Should_find_prefer_constants_file_on_disk_over_embedded_if_both_exist() { var middleware = new StaticFileMiddleware(new DummyNext()); var context = new OwinContext { Request = { - Path = new PathString("/nOie.html"), + Path = new PathString("/js/app.constants.js"), //this exists both BOTH embedded in ServicePulse.Host.exe and on disk Method = "GET" } }; middleware.Invoke(context); - const long sizeOfEmbeddedHtmlFile = 1302; // this is the NoIe.html file embedded into ServicePulse.Host.exe - Assert.AreEqual(sizeOfEmbeddedHtmlFile, context.Response.ContentLength); - Assert.AreEqual(("text/html"), context.Response.ContentType); + const long sizeOfFileOnDisk = 231; // this is the /app/js/app.constants.js file + Assert.AreEqual(sizeOfFileOnDisk, context.Response.ContentLength); + Assert.AreEqual(("application/javascript"), context.Response.ContentType); } - [Test] - public void Should_find_deep_linking_file_embedded_in_assembly() + public void Should_not_find_other_files_on_disk() { var middleware = new StaticFileMiddleware(new DummyNext()); var context = new OwinContext { Request = { - Path = new PathString("/js/views/message/editor/messageEditorModal.controller.js"), + Path = new PathString("/filename.js"), Method = "GET" } }; middleware.Invoke(context); - const long sizeOfEmbeddedHtmlFile = 8544; // this is the messageEditorModal.controller.js file embedded into ServicePulse.Host.exe - Assert.AreEqual(sizeOfEmbeddedHtmlFile, context.Response.ContentLength); - Assert.AreEqual(("application/javascript"), context.Response.ContentType); - } + Assert.AreEqual(null, context.Response.ContentLength); + Assert.AreEqual(null, context.Response.ContentType); - [Test] - public void Should_find_prefer_file_on_disk_over_embedded_if_both_exist() - { - var middleware = new StaticFileMiddleware(new DummyNext()); - var context = new OwinContext + context = new OwinContext { Request = { - Path = new PathString("/index.html"), //this exists both BOTH embedded in ServicePulse.Host.exe and on disk + Path = new PathString("/../ServicePulse.Host.exe.config"), Method = "GET" } }; middleware.Invoke(context); - const long sizeOfFileOnDisk = 23; // this is the /app/filename.js file - Assert.AreEqual(sizeOfFileOnDisk, context.Response.ContentLength); - Assert.AreEqual(("text/html"), context.Response.ContentType); + Assert.AreEqual(null, context.Response.ContentLength); + Assert.AreEqual(null, context.Response.ContentType); } public class DummyNext : OwinMiddleware diff --git a/src/ServicePulse.Host.Tests/ServicePulse.Host.Tests.csproj b/src/ServicePulse.Host.Tests/ServicePulse.Host.Tests.csproj index b404fb12a1..9561f52afa 100644 --- a/src/ServicePulse.Host.Tests/ServicePulse.Host.Tests.csproj +++ b/src/ServicePulse.Host.Tests/ServicePulse.Host.Tests.csproj @@ -21,13 +21,11 @@ - - PreserveNewest - PreserveNewest - + + %(RelativeDir)%(Filename)%(Extension) PreserveNewest diff --git a/src/ServicePulse.Host.Tests/app/filename.unknown b/src/ServicePulse.Host.Tests/app/filename.unknown deleted file mode 100644 index 5f282702bb..0000000000 --- a/src/ServicePulse.Host.Tests/app/filename.unknown +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/src/ServicePulse.Host.Tests/app/index.html b/src/ServicePulse.Host.Tests/app/index.html deleted file mode 100644 index 60a5544d1b..0000000000 --- a/src/ServicePulse.Host.Tests/app/index.html +++ /dev/null @@ -1 +0,0 @@ -index.html from disk \ No newline at end of file diff --git a/src/ServicePulse.Host.Tests/app/js/app.constants.js b/src/ServicePulse.Host.Tests/app/js/app.constants.js new file mode 100644 index 0000000000..83e5a4080e --- /dev/null +++ b/src/ServicePulse.Host.Tests/app/js/app.constants.js @@ -0,0 +1,7 @@ +window.defaultConfig = { + default_route: '/dashboard', + version: '1.2.0', + service_control_url: 'http://some.host.com:33333/api/', + monitoring_urls: ['http://some.host.com:33633/'], + showPendingRetry: true +}; diff --git a/src/ServicePulse.Host/Owin/FileOnDiskFinder.cs b/src/ServicePulse.Host/Owin/FileOnDiskFinder.cs index 861e1689a2..da3e485758 100644 --- a/src/ServicePulse.Host/Owin/FileOnDiskFinder.cs +++ b/src/ServicePulse.Host/Owin/FileOnDiskFinder.cs @@ -6,13 +6,15 @@ namespace ServicePulse.Host.Owin { public static class FileOnDiskFinder { + static readonly string appConstantsPath = Path.GetFullPath(Path.Combine(AppDomain.CurrentDomain.BaseDirectory,"app","js","app.constants.js")); + public static IFileInfo FindFile(string filePath) { var fileWithFullDirectoryPath = Path.GetFullPath(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, filePath)); IFileInfo fileInfo = null; - if (File.Exists(fileWithFullDirectoryPath)) + if (fileWithFullDirectoryPath.Equals(appConstantsPath, StringComparison.OrdinalIgnoreCase) && File.Exists(fileWithFullDirectoryPath)) { fileInfo = new PhysicalFileInfo(new FileInfo(fileWithFullDirectoryPath)); }