Skip to content
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.

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

Fix secondary order XSS: Harden file type inputs #4635

Merged
merged 1 commit into from Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions BTCPayServer.Tests/FastTests.cs
Expand Up @@ -581,6 +581,22 @@ private DateTimeOffset Date(int days)
return new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero) + TimeSpan.FromDays(days);
}

[Fact]
public void CanDetectImage()
{
Assert.True(FileTypeDetector.IsPicture(new byte[] { 0x42, 0x4D }, "test.bmp"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { 0x42, 0x4D }, ".bmp"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { 0x42, 0x4D }, "test.svg"));
Assert.True(FileTypeDetector.IsPicture(new byte[] { 0xFF, 0xD8, 0xFF, 0xD9 }, "test.jpg"));
Assert.True(FileTypeDetector.IsPicture(new byte[] { 0xFF, 0xD8, 0xFF, 0xD9 }, "test.jpeg"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { 0xFF, 0xD8, 0xFF, 0xDA }, "test.jpg"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { 0xFF, 0xD8, 0xFF }, "test.jpg"));
Assert.True(FileTypeDetector.IsPicture(new byte[] { 0x3C, 0x73, 0x76, 0x67 }, "test.svg"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { 0x3C, 0x73, 0x76, 0x67 }, "test.jpg"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { 0xFF }, "e.jpg"));
Assert.False(FileTypeDetector.IsPicture(new byte[] { }, "empty.jpg"));
}

[Fact]
public void RoundupCurrenciesCorrectly()
{
Expand Down
62 changes: 62 additions & 0 deletions BTCPayServer/BufferizedFormFile.cs
@@ -0,0 +1,62 @@
using Microsoft.AspNetCore.Http;
using System.IO;
using System.Threading;
using System.Threading.Tasks;

namespace BTCPayServer
{
public class BufferizedFormFile : IFormFile
{
private IFormFile _formFile;
private MemoryStream _content;
public byte[] Buffer { get; }
BufferizedFormFile(IFormFile formFile, byte[] content)
{
_formFile = formFile;
Buffer = content;
_content = new MemoryStream(content);
}

public string ContentType => _formFile.ContentType;

public string ContentDisposition => _formFile.ContentDisposition;

public IHeaderDictionary Headers => _formFile.Headers;

public long Length => _formFile.Length;

public string Name => _formFile.Name;

public string FileName => _formFile.FileName;

public static async Task<BufferizedFormFile> Bufferize(IFormFile formFile)
{
if (formFile is BufferizedFormFile b)
return b;
var content = new byte[formFile.Length];
using var fs = formFile.OpenReadStream();
await fs.ReadAsync(content, 0, content.Length);
return new BufferizedFormFile(formFile, content);
}

public void CopyTo(Stream target)
{
_content.CopyTo(target);
}

public Task CopyToAsync(Stream target, CancellationToken cancellationToken = default)
{
return _content.CopyToAsync(target, cancellationToken);
}

public Stream OpenReadStream()
{
return _content;
}

public void Rewind()
{
_content.Seek(0, SeekOrigin.Begin);
}
}
}
49 changes: 31 additions & 18 deletions BTCPayServer/Controllers/UIServerController.cs
Expand Up @@ -1043,30 +1043,43 @@ public async Task<IActionResult> Theme()

if (model.LogoFile != null)
{
if (model.LogoFile.ContentType.StartsWith("image/", StringComparison.InvariantCulture))
if (model.LogoFile.Length > 1_000_000)
{
// delete existing image
if (!string.IsNullOrEmpty(settings.LogoFileId))
{
await _fileService.RemoveFile(settings.LogoFileId, userId);
}

// add new image
try
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file should be less than 1MB";
}
else if (!model.LogoFile.ContentType.StartsWith("image/", StringComparison.InvariantCulture))
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file needs to be an image";
}
else
{
var formFile = await model.LogoFile.Bufferize();
if (!FileTypeDetector.IsPicture(formFile.Buffer, formFile.FileName))
{
var storedFile = await _fileService.AddFile(model.LogoFile, userId);
settings.LogoFileId = storedFile.Id;
settingsChanged = true;
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file needs to be an image";
}
catch (Exception e)
else
{
ModelState.AddModelError(nameof(settings.LogoFile), $"Could not save logo: {e.Message}");
model.LogoFile = formFile;
// delete existing image
if (!string.IsNullOrEmpty(settings.LogoFileId))
{
await _fileService.RemoveFile(settings.LogoFileId, userId);
}

// add new image
try
{
var storedFile = await _fileService.AddFile(model.LogoFile, userId);
settings.LogoFileId = storedFile.Id;
settingsChanged = true;
}
catch (Exception e)
{
ModelState.AddModelError(nameof(settings.LogoFile), $"Could not save logo: {e.Message}");
}
}
}
else
{
ModelState.AddModelError(nameof(settings.LogoFile), "The uploaded logo file needs to be an image");
}
}
else if (RemoveLogoFile && !string.IsNullOrEmpty(settings.LogoFileId))
{
Expand Down
66 changes: 44 additions & 22 deletions BTCPayServer/Controllers/UIStoresController.cs
Expand Up @@ -27,6 +27,7 @@
using BTCPayServer.Services.Wallets;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Rendering;
Expand Down Expand Up @@ -658,29 +659,42 @@ public IActionResult GeneralSettings()

if (model.LogoFile != null)
{
if (model.LogoFile.ContentType.StartsWith("image/", StringComparison.InvariantCulture))
if (model.LogoFile.Length > 1_000_000)
{
// delete existing image
if (!string.IsNullOrEmpty(blob.LogoFileId))
{
await _fileService.RemoveFile(blob.LogoFileId, userId);
}

// add new image
try
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file should be less than 1MB";
}
else if (!model.LogoFile.ContentType.StartsWith("image/", StringComparison.InvariantCulture))
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file needs to be an image";
}
else
{
var formFile = await model.LogoFile.Bufferize();
if (!FileTypeDetector.IsPicture(formFile.Buffer, formFile.FileName))
{
var storedFile = await _fileService.AddFile(model.LogoFile, userId);
blob.LogoFileId = storedFile.Id;
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file needs to be an image";
}
catch (Exception e)
else
{
TempData[WellKnownTempData.ErrorMessage] = $"Could not save logo: {e.Message}";
model.LogoFile = formFile;
// delete existing image
if (!string.IsNullOrEmpty(blob.LogoFileId))
{
await _fileService.RemoveFile(blob.LogoFileId, userId);
}

// add new image
try
{
var storedFile = await _fileService.AddFile(model.LogoFile, userId);
blob.LogoFileId = storedFile.Id;
}
catch (Exception e)
{
TempData[WellKnownTempData.ErrorMessage] = $"Could not save logo: {e.Message}";
}
}
}
else
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded logo file needs to be an image";
}
}
else if (RemoveLogoFile && !string.IsNullOrEmpty(blob.LogoFileId))
{
Expand All @@ -691,7 +705,19 @@ public IActionResult GeneralSettings()

if (model.CssFile != null)
{
if (model.CssFile.ContentType.Equals("text/css", StringComparison.InvariantCulture))
if (model.CssFile.Length > 1_000_000)
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded file should be less than 1MB";
Comment on lines +708 to +710
Copy link
Member

Choose a reason for hiding this comment

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

We can lower this limit here as 100KB should already be more than enough. (Bootstrap as a whole is > 300KB)

}
else if (!model.CssFile.ContentType.Equals("text/css", StringComparison.InvariantCulture))
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded file needs to be a CSS file";
}
else if (!model.CssFile.FileName.EndsWith(".css", StringComparison.OrdinalIgnoreCase))
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded file needs to be a CSS file";
}
else
{
// delete existing CSS file
if (!string.IsNullOrEmpty(blob.CssFileId))
Expand All @@ -710,10 +736,6 @@ public IActionResult GeneralSettings()
TempData[WellKnownTempData.ErrorMessage] = $"Could not save CSS file: {e.Message}";
}
}
else
{
TempData[WellKnownTempData.ErrorMessage] = "The uploaded file needs to be a CSS file";
}
}
else if (RemoveCssFile && !string.IsNullOrEmpty(blob.CssFileId))
{
Expand Down
4 changes: 4 additions & 0 deletions BTCPayServer/Extensions.cs
Expand Up @@ -36,6 +36,10 @@ namespace BTCPayServer
{
public static class Extensions
{
public static Task<BufferizedFormFile> Bufferize(this IFormFile formFile)
{
return BufferizedFormFile.Bufferize(formFile);
}
/// <summary>
/// Unescape Uri string for %2F
/// See details at: https://github.com/dotnet/aspnetcore/issues/14170#issuecomment-533342396
Expand Down
92 changes: 92 additions & 0 deletions BTCPayServer/FileTypeDetector.cs
@@ -0,0 +1,92 @@
#nullable enable
using System;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using NBitcoin.DataEncoders;

namespace BTCPayServer
{
public class FileTypeDetector
{
// Thanks to https://www.garykessler.net/software/FileSigs_20220731.zip

const string pictureSigs =
"JPEG2000 image files,00 00 00 0C 6A 50 20 20,JP2,Picture,0,(null)\n" +
"Bitmap image,42 4D,BMP|DIB,Picture,0,(null)\n" +
"GIF file,47 49 46 38,GIF,Picture,0,00 3B\n" +
"PNG image,89 50 4E 47 0D 0A 1A 0A,PNG|APNG,Picture,0,49 45 4E 44 AE 42 60 82\n" +
"Generic JPEGimage fil,FF D8,JPE|JPEG|JPG,Picture,0,FF D9\n" +
"JPEG-EXIF-SPIFF images,FF D8 FF,JFIF|JPE|JPEG|JPG,Picture,0,FF D9\n" +
"SVG images, 3C 73 76 67,SVG,Picture,0,(null)\n" +
"Google WebP image file, 52 49 46 46 XX XX XX XX 57 45 42 50,WEBP,Picture,0,(null)\n" +
"AVIF image file, XX XX XX XX 66 74 79 70,AVIF,Picture,0,(null)\n";

readonly static (int[] Header, int[]? Trailer, string[] Extensions)[] headerTrailers;
static FileTypeDetector()
{
var lines = pictureSigs.Split('\n', StringSplitOptions.RemoveEmptyEntries);
headerTrailers = new (int[] Header, int[]? Trailer, string[] Extensions)[lines.Length];
for (int i = 0; i < lines.Length; i++)
{
var cells = lines[i].Split(',');
headerTrailers[i] = (
DecodeData(cells[1]),
cells[^1] == "(null)" ? null : DecodeData(cells[^1]),
cells[2].Split('|').Select(p => $".{p}").ToArray()
);
}
}

private static int[] DecodeData(string pattern)
{
pattern = pattern.Replace(" ", "");
int[] res = new int[pattern.Length / 2];
for (int i = 0; i < pattern.Length; i+=2)
{
var b = pattern[i..(i + 2)];
if (b == "XX")
res[i/2] = -1;
else
res[i/2] = byte.Parse(b, System.Globalization.NumberStyles.HexNumber);
}
return res;
}

public static bool IsPicture(byte[] bytes, string? filename)
{
for (int i = 0; i < headerTrailers.Length; i++)
{
if (headerTrailers[i].Header is int[] header)
{
if (header.Length > bytes.Length)
goto next;
for (int x = 0; x < header.Length; x++)
{
if (bytes[x] != header[x] && header[x] != -1)
goto next;
}
}
if (headerTrailers[i].Trailer is int[] trailer)
{
if (trailer.Length > bytes.Length)
goto next;
for (int x = 0; x < trailer.Length; x++)
{
if (bytes[^(trailer.Length - x)] != trailer[x] && trailer[x] != -1)
goto next;
}
}

if (filename is not null)
{
if (!headerTrailers[i].Extensions.Any(ext => filename.Length > ext.Length && filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase)))
return false;
}
return true;
next:
;
}
return false;
}
}
}
1 change: 1 addition & 0 deletions BTCPayServer/Storage/StorageExtensions.cs
Expand Up @@ -76,6 +76,7 @@ private static Action<StaticFileResponseContext> HandleStaticFileResponse()
context.Context.Response.Headers["Content-Disposition"] = "attachment";
}
context.Context.Response.Headers["Content-Security-Policy"] = "script-src ;";
context.Context.Response.Headers["X-Content-Type-Options"] = "nosniff";
};
}
}
Expand Down