Skip to content

Resolve directory traversal vulnerability in static file middleware #831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 11, 2020
Merged

Resolve directory traversal vulnerability in static file middleware #831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 45 additions & 53 deletions src/ServicePulse.Host.Tests/Owin/StaticMiddlewareTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Contributor

@WojcikMike WojcikMike Mar 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikeminutillo @janovesk is there any reason that this test was not removed?

Copy link
Member Author

@mikeminutillo mikeminutillo Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention was to put something back in it's place after the security advisory was published. Maybe an approval test of all extensions in the assembly along with the mime-type or something.

//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()
{
Expand All @@ -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"
}
};
Expand Down Expand Up @@ -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
}
};
Expand All @@ -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
Expand Down
6 changes: 2 additions & 4 deletions src/ServicePulse.Host.Tests/ServicePulse.Host.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
</ItemGroup>

<ItemGroup>
<None Update="app\index.html">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="app\filename.js">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="app\filename.unknown">
<None Update="app\js\app.constants.js">
<LogicalName>%(RelativeDir)%(Filename)%(Extension)</LogicalName>
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
</ItemGroup>
Expand Down
1 change: 0 additions & 1 deletion src/ServicePulse.Host.Tests/app/filename.unknown
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/ServicePulse.Host.Tests/app/index.html
View file
Open in desktop

This file was deleted.

7 changes: 7 additions & 0 deletions src/ServicePulse.Host.Tests/app/js/app.constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
};
4 changes: 3 additions & 1 deletion src/ServicePulse.Host/Owin/FileOnDiskFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down