Skip to content

Commit

Permalink
Merge pull request from GHSA-h2wr-3c7f-7h79
Browse files Browse the repository at this point in the history
* Enforce SHA1 hash regex upon all asset-related API endpoints

* Add unit tests for asset uploading & retrieval
  • Loading branch information
jvyden authored Sep 24, 2023
1 parent fc034c5 commit be897f2
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Refresh.GameServer.Extensions;
using Refresh.GameServer.Types.Roles;
using Refresh.GameServer.Types.UserData;
using Refresh.GameServer.Verification;

namespace Refresh.GameServer.Endpoints.ApiV3.Admin;

Expand Down Expand Up @@ -50,7 +51,7 @@ public ApiListResponse<ApiExtendedGameUserResponse> GetExtendedUsers(RequestCont

private static ApiOkResponse ResetUserPassword(GameDatabaseContext database, ApiResetUserPasswordRequest body, GameUser user)
{
if (body.PasswordSha512.Length != 128 || !AuthenticationApiEndpoints.Sha512Regex().IsMatch(body.PasswordSha512))
if (body.PasswordSha512.Length != 128 || !CommonPatterns.Sha512Regex().IsMatch(body.PasswordSha512))
return new ApiValidationError("Password is definitely not SHA512. Please hash the password.");

string? passwordBcrypt = BC.HashPassword(body.PasswordSha512, AuthenticationApiEndpoints.WorkFactor);
Expand Down
16 changes: 5 additions & 11 deletions Refresh.GameServer/Endpoints/ApiV3/AuthenticationApiEndpoints.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Net;
using System.Text.RegularExpressions;
using AttribDoc.Attributes;
using Bunkum.CustomHttpListener.Parsing;
using Bunkum.HttpServer;
Expand All @@ -17,10 +16,11 @@
using Refresh.GameServer.Services;
using Refresh.GameServer.Types.Roles;
using Refresh.GameServer.Types.UserData;
using Refresh.GameServer.Verification;

namespace Refresh.GameServer.Endpoints.ApiV3;

public partial class AuthenticationApiEndpoints : EndpointGroup
public class AuthenticationApiEndpoints : EndpointGroup
{
// How many rounds to do for password hashing (BCrypt)
// 14 is ~1 second for logins and reset, which is fair because logins are a one-time thing
Expand All @@ -31,12 +31,6 @@ public partial class AuthenticationApiEndpoints : EndpointGroup
// If decreased, passwords will stay at higher WorkFactor until reset
public const int WorkFactor = 14;

[GeneratedRegex("^[a-f0-9]{128}$")]
public static partial Regex Sha512Regex();

[GeneratedRegex("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+[.][a-zA-Z]{2,}$")]
private static partial Regex EmailAddressRegex();

[ApiV3Endpoint("login", Method.Post), Authentication(false), AllowDuringMaintenance]
[DocRequestBody(typeof(ApiAuthenticationRequest))]
[RateLimitSettings(300, 10, 300, "auth")]
Expand Down Expand Up @@ -117,7 +111,7 @@ public ApiOkResponse ResetPassword(RequestContext context, GameDatabaseContext d
user ??= database.GetUserFromTokenData(body.ResetToken, TokenType.PasswordReset);
if (user == null) return new ApiAuthenticationError("The reset token is invalid");

if (body.PasswordSha512.Length != 128 || !Sha512Regex().IsMatch(body.PasswordSha512))
if (body.PasswordSha512.Length != 128 || !CommonPatterns.Sha512Regex().IsMatch(body.PasswordSha512))
return new ApiValidationError("Password is definitely not SHA512. Please hash the password.");

string? passwordBcrypt = BC.HashPassword(body.PasswordSha512, WorkFactor);
Expand Down Expand Up @@ -192,10 +186,10 @@ public ApiResponse<IApiAuthenticationResponse> Register(RequestContext context,
if (!config.RegistrationEnabled)
return new ApiAuthenticationError("Registration is not enabled on this server.");

if (body.PasswordSha512.Length != 128 || !Sha512Regex().IsMatch(body.PasswordSha512))
if (body.PasswordSha512.Length != 128 || !CommonPatterns.Sha512Regex().IsMatch(body.PasswordSha512))
return new ApiValidationError("Password is definitely not SHA512. Please hash the password.");

if (!EmailAddressRegex().IsMatch(body.EmailAddress))
if (!CommonPatterns.EmailAddressRegex().IsMatch(body.EmailAddress))
return new ApiValidationError("The email address given is invalid.");

if (database.IsUsernameTaken(body.Username) || database.IsEmailTaken(body.EmailAddress))
Expand Down
8 changes: 8 additions & 0 deletions Refresh.GameServer/Endpoints/ApiV3/ResourceApiEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Refresh.GameServer.Endpoints.ApiV3.DataTypes.Response;
using Refresh.GameServer.Importing;
using Refresh.GameServer.Types.Assets;
using Refresh.GameServer.Verification;

namespace Refresh.GameServer.Endpoints.ApiV3;

Expand All @@ -18,6 +19,10 @@ public class ResourceApiEndpoints : EndpointGroup
private const string HashMissingErrorWhen = "The hash is missing or null";
private static readonly ApiValidationError HashMissingError = new(HashMissingErrorWhen);
private static readonly Response HashMissingErrorResponse = HashMissingError;

private const string HashInvalidErrorWhen = "The hash is invalid (should be SHA1 hash)";
private static readonly ApiValidationError HashInvalidError = new(HashInvalidErrorWhen);
private static readonly Response HashInvalidErrorResponse = HashInvalidError;

private const string CouldNotGetAssetErrorWhen = "An error occurred while retrieving the asset from the data store";
private static readonly ApiInternalError CouldNotGetAssetError = new(CouldNotGetAssetErrorWhen);
Expand All @@ -36,6 +41,7 @@ public class ResourceApiEndpoints : EndpointGroup
public Response DownloadGameAsset(RequestContext context, IDataStore dataStore,
[DocSummary("The SHA1 hash of the asset")] string hash)
{
if (!CommonPatterns.Sha1Regex().IsMatch(hash)) return HashInvalidError;
if (string.IsNullOrWhiteSpace(hash)) return HashMissingErrorResponse;
if (!dataStore.ExistsInStore(hash)) return ApiNotFoundError.Instance;

Expand All @@ -54,6 +60,7 @@ public Response DownloadGameAsset(RequestContext context, IDataStore dataStore,
public Response DownloadGameAssetAsImage(RequestContext context, IDataStore dataStore, GameDatabaseContext database,
[DocSummary("The SHA1 hash of the asset")] string hash)
{
if (!CommonPatterns.Sha1Regex().IsMatch(hash)) return HashInvalidError;
if (string.IsNullOrWhiteSpace(hash)) return HashMissingErrorResponse;
if (!dataStore.ExistsInStore(hash)) return ApiNotFoundError.Instance;

Expand All @@ -79,6 +86,7 @@ public Response DownloadGameAssetAsImage(RequestContext context, IDataStore data
public ApiResponse<ApiGameAssetResponse> GetAssetInfo(RequestContext context, GameDatabaseContext database,
[DocSummary("The SHA1 hash of the asset")] string hash)
{
if (!CommonPatterns.Sha1Regex().IsMatch(hash)) return HashInvalidError;
if (string.IsNullOrWhiteSpace(hash)) return HashMissingError;

GameAsset? asset = database.GetAssetFromHash(hash);
Expand Down
13 changes: 11 additions & 2 deletions Refresh.GameServer/Endpoints/Game/ResourceEndpoints.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Text.RegularExpressions;
using Bunkum.CustomHttpListener.Parsing;
using Bunkum.HttpServer;
using Bunkum.HttpServer.Endpoints;
Expand All @@ -15,6 +16,7 @@
using Refresh.GameServer.Types.Lists;
using Refresh.GameServer.Types.Roles;
using Refresh.GameServer.Types.UserData;
using Refresh.GameServer.Verification;

namespace Refresh.GameServer.Endpoints.Game;

Expand All @@ -27,8 +29,9 @@ public class ResourceEndpoints : EndpointGroup
public Response UploadAsset(RequestContext context, string hash, string type, byte[] body, IDataStore dataStore,
GameDatabaseContext database, GameUser user, AssetImporter importer, GameServerConfig config, IDateTimeProvider timeProvider, Token token)
{
if (!CommonPatterns.Sha1Regex().IsMatch(hash)) return BadRequest;

bool isPSP = context.IsPSP();

string assetPath = isPSP ? $"psp/{hash}" : hash;

if (dataStore.ExistsInStore(assetPath))
Expand Down Expand Up @@ -63,6 +66,8 @@ public Response UploadAsset(RequestContext context, string hash, string type, by
[MinimumRole(GameUserRole.Restricted)]
public Response GetResource(RequestContext context, string hash, IDataStore dataStore, GameDatabaseContext database, Token token)
{
if (!CommonPatterns.Sha1Regex().IsMatch(hash)) return BadRequest;

//If the request comes from a PSP client,
if (context.IsPSP())
{
Expand All @@ -83,7 +88,8 @@ public Response GetResource(RequestContext context, string hash, IDataStore data
[GameEndpoint("showNotUploaded", Method.Post, ContentType.Xml)]
[GameEndpoint("filterResources", Method.Post, ContentType.Xml)]
[MinimumRole(GameUserRole.Restricted)]
public SerializedResourceList GetAssetsMissingFromStore(RequestContext context, SerializedResourceList body, IDataStore dataStore)
[NullStatusCode(BadRequest)]
public SerializedResourceList? GetAssetsMissingFromStore(RequestContext context, SerializedResourceList body, IDataStore dataStore)
{
if (context.IsPSP())
{
Expand All @@ -96,6 +102,9 @@ public SerializedResourceList GetAssetsMissingFromStore(RequestContext context,
}
}

if(body.Items.Any(hash => !CommonPatterns.Sha1Regex().IsMatch(hash)))
return null;

return new SerializedResourceList(body.Items.Where(r => !dataStore.ExistsInStore(r)));
}
}
12 changes: 9 additions & 3 deletions Refresh.GameServer/Importing/AssetImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,20 @@
using NotEnoughLogs;
using Refresh.GameServer.Authentication;
using Refresh.GameServer.Database;
using Refresh.GameServer.Time;
using Refresh.GameServer.Types.Assets;

namespace Refresh.GameServer.Importing;

public class AssetImporter : Importer
{
public AssetImporter(LoggerContainer<BunkumContext>? logger = null) : base(logger)
{}
private readonly IDateTimeProvider _timeProvider;

public AssetImporter(LoggerContainer<BunkumContext>? logger = null, IDateTimeProvider? timeProvider = null) : base(logger)
{
timeProvider ??= new SystemDateTimeProvider();
this._timeProvider = timeProvider;
}

public void ImportFromDataStoreCli(GameDatabaseContext context, IDataStore dataStore)
{
Expand Down Expand Up @@ -80,7 +86,7 @@ public void ImportFromDataStore(GameDatabaseContext context, IDataStore dataStor

GameAsset asset = new()
{
UploadDate = DateTimeOffset.Now,
UploadDate = this._timeProvider.Now,
OriginalUploader = null,
AssetHash = hash,
AssetType = DetermineAssetType(data, platform),
Expand Down
1 change: 1 addition & 0 deletions Refresh.GameServer/Importing/Importer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ protected void Warn(string message)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool MatchesMagic(ReadOnlySpan<byte> data, ReadOnlySpan<byte> magic)
{
if (magic.Length > data.Length) return false;
return data[..magic.Length].SequenceEqual(magic);
}

Expand Down
5 changes: 3 additions & 2 deletions Refresh.GameServer/Services/ImportService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
using Bunkum.HttpServer.Services;
using NotEnoughLogs;
using Refresh.GameServer.Importing;
using Refresh.GameServer.Time;

namespace Refresh.GameServer.Services;

public class ImportService : Service
{
internal ImportService(LoggerContainer<BunkumContext> logger) : base(logger)
internal ImportService(LoggerContainer<BunkumContext> logger, TimeProviderService timeProvider) : base(logger)
{
this._assetImporter = new AssetImporter(logger);
this._assetImporter = new AssetImporter(logger, timeProvider.TimeProvider);
this._imageImporter = new ImageImporter(logger);
}

Expand Down
7 changes: 3 additions & 4 deletions Refresh.GameServer/Services/TimeProviderService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,18 @@ namespace Refresh.GameServer.Services;

public class TimeProviderService : Service
{
private readonly IDateTimeProvider _timeProvider;
public IDateTimeProvider TimeProvider { get; }

internal TimeProviderService(LoggerContainer<BunkumContext> logger, IDateTimeProvider timeProvider) : base(logger)
{
this._timeProvider = timeProvider;

this.TimeProvider = timeProvider;
}

public override object? AddParameterToEndpoint(ListenerContext context, ParameterInfo paramInfo, Lazy<IDatabaseContext> database)
{
if (paramInfo.ParameterType == typeof(IDateTimeProvider))
{
return this._timeProvider;
return this.TimeProvider;
}

return null;
Expand Down
15 changes: 15 additions & 0 deletions Refresh.GameServer/Verification/CommonPatterns.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.Text.RegularExpressions;

namespace Refresh.GameServer.Verification;

public static partial class CommonPatterns
{
[GeneratedRegex("^[a-f0-9]{40}$")]
public static partial Regex Sha1Regex();

[GeneratedRegex("^[a-f0-9]{128}$")]
public static partial Regex Sha512Regex();

[GeneratedRegex("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+[.][a-zA-Z]{2,}$")]
public static partial Regex EmailAddressRegex();
}
5 changes: 4 additions & 1 deletion RefreshTests.GameServer/GameServer/TestRefreshGameServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
using Refresh.GameServer;
using Refresh.GameServer.Configuration;
using Refresh.GameServer.Database;
using Refresh.GameServer.Importing;
using Refresh.GameServer.Services;
using Refresh.GameServer.Time;
using Refresh.GameServer.Types.Assets;
using RefreshTests.GameServer.Time;

namespace RefreshTests.GameServer.GameServer;
Expand Down Expand Up @@ -42,6 +45,6 @@ protected override void SetupMiddlewares()

protected override void SetupServices()
{

this._server.AddService<TimeProviderService>(this.DateTimeProvider);
}
}
1 change: 1 addition & 0 deletions RefreshTests.GameServer/RefreshTests.GameServer.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@
<PackageReference Include="NotEnoughLogs" Version="1.1.0" />
</ItemGroup>


</Project>
64 changes: 64 additions & 0 deletions RefreshTests.GameServer/Tests/Assets/AssetUploadTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using System.Security.Cryptography;
using Refresh.GameServer.Authentication;
using Refresh.GameServer.Services;
using Refresh.GameServer.Types.UserData;

namespace RefreshTests.GameServer.Tests.Assets;

public class AssetUploadTests : GameServerTest
{
[Test]
public void CanUploadAsset()
{
using TestContext context = this.GetServer();
context.Server.Value.Server.AddService<ImportService>();
GameUser user = context.CreateUser();
using HttpClient client = context.GetAuthenticatedClient(TokenType.Game, user);

ReadOnlySpan<byte> data = "TEX a"u8;

string hash = BitConverter.ToString(SHA1.HashData(data))
.Replace("-", "")
.ToLower();

HttpResponseMessage response = client.PostAsync("/lbp/upload/" + hash, new ByteArrayContent(data.ToArray())).Result;
Assert.That(response.IsSuccessStatusCode, Is.True);
Assert.That(response.StatusCode, Is.EqualTo(OK));
}

[Test]
public void CanRetrieveAsset()
{
using TestContext context = this.GetServer();
context.Server.Value.Server.AddService<ImportService>();
GameUser user = context.CreateUser();
using HttpClient client = context.GetAuthenticatedClient(TokenType.Game, user);

ReadOnlySpan<byte> data = "TEX a"u8;
string hash = BitConverter.ToString(SHA1.HashData(data))
.Replace("-", "")
.ToLower();

client.PostAsync("/lbp/upload/" + hash, new ByteArrayContent(data.ToArray())).Wait();
HttpResponseMessage response = client.GetAsync("/lbp/r/" + hash).Result;
Assert.That(response.IsSuccessStatusCode, Is.True);
byte[] returnedData = response.Content.ReadAsByteArrayAsync().Result;

Assert.That(data.SequenceEqual(returnedData), Is.True);
}

[Test]
public void InvalidHashFails()
{
using TestContext context = this.GetServer();
context.Server.Value.Server.AddService<ImportService>();
GameUser user = context.CreateUser();
using HttpClient client = context.GetAuthenticatedClient(TokenType.Game, user);

HttpResponseMessage response = client.GetAsync("/lbp/r/asdf").Result;
Assert.That(response.StatusCode, Is.EqualTo(BadRequest));

response = client.GetAsync("/lbp/r/..%2Frpc.json").Result;
Assert.That(response.StatusCode, Is.EqualTo(BadRequest));
}
}
2 changes: 1 addition & 1 deletion RefreshTests.GameServer/Time/MockDateTimeProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ public class MockDateTimeProvider : IDateTimeProvider
{
public long TimestampMilliseconds { get; set; }
public long TimestampSeconds => TimestampMilliseconds / 1000;
public long EarliestDate => new DateTimeOffset(2007, 1, 1, 0, 0, 0, TimeSpan.Zero).ToUnixTimeSeconds();
public long EarliestDate => 0;
public DateTimeOffset Now => DateTimeOffset.FromUnixTimeMilliseconds(TimestampMilliseconds);
}

0 comments on commit be897f2

Please sign in to comment.