Fix doubly sanitized strings (#727)

* Add migration to de-sanitize database strings

* Remove SanitizationHelper functions related to XML sanitization

* Remove sanitization usage from website

* Implement suggested changes
This commit is contained in:
Josh 2023-03-30 18:03:08 -05:00 committed by GitHub
parent f5c8f53437
commit 50d1d9c7e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 26 additions and 66 deletions

View file

@ -34,8 +34,6 @@ public class FriendsController : ControllerBase
NPData? npData = await this.DeserializeBody<NPData>();
if (npData == null) return this.BadRequest();
SanitizationHelper.SanitizeStringsInClass(npData);
List<UserEntity> friends = new();
foreach (string friendName in npData.Friends ?? new List<string>())
{

View file

@ -37,8 +37,6 @@ public class ReportController : ControllerBase
GameGriefReport? report = await this.DeserializeBody<GameGriefReport>();
if (report == null) return this.BadRequest();
SanitizationHelper.SanitizeStringsInClass(report);
if (string.IsNullOrWhiteSpace(report.JpegHash)) return this.BadRequest();
if (!FileHelper.ResourceExists(report.JpegHash)) return this.BadRequest();

View file

@ -42,8 +42,6 @@ public class PhotosController : ControllerBase
GamePhoto? photo = await this.DeserializeBody<GamePhoto>();
if (photo == null) return this.BadRequest();
SanitizationHelper.SanitizeStringsInClass(photo);
foreach (PhotoEntity p in this.database.Photos.Where(p => p.CreatorId == user.UserId))
{
if (p.LargeHash == photo.LargeHash) return this.Ok(); // photo already uplaoded

View file

@ -95,8 +95,6 @@ public class ScoreController : ControllerBase
return this.BadRequest();
}
SanitizationHelper.SanitizeStringsInClass(score);
int slotId = id;
if (slotType == "developer") slotId = await SlotHelper.GetPlaceholderSlotId(this.database, slotId, SlotType.Developer);

View file

@ -72,8 +72,6 @@ public class UserController : ControllerBase
if (update == null) return this.BadRequest();
SanitizationHelper.SanitizeStringsInClass(update);
if (update.Biography != null)
{
if (update.Biography.Length > 512) return this.BadRequest();

View file

@ -68,8 +68,6 @@ public class SlotPageController : ControllerBase
return this.Redirect("~/slot/" + id);
}
// Prevent potential xml injection and censor content
msg = SanitizationHelper.SanitizeString(msg);
msg = CensorHelper.FilterMessage(msg);
bool success = await this.database.PostComment(token.UserId, id, CommentType.Level, msg);

View file

@ -44,8 +44,6 @@ public class UserPageController : ControllerBase
return this.Redirect("~/user/" + id);
}
// Prevent potential xml injection and censor content
msg = SanitizationHelper.SanitizeString(msg);
msg = CensorHelper.FilterMessage(msg);
bool success = await this.database.PostComment(token.UserId, id, CommentType.Profile, msg);

View file

@ -29,15 +29,13 @@ public class SlotSettingsPage : BaseLayout
if (avatarHash != null) this.Slot.IconHash = avatarHash;
name = SanitizationHelper.SanitizeString(name);
name = CensorHelper.FilterMessage(name);
if (this.Slot.Name != name && name.Length <= 64) this.Slot.Name = name;
description = SanitizationHelper.SanitizeString(description);
description = CensorHelper.FilterMessage(description);
if (this.Slot.Description != description && description.Length <= 512) this.Slot.Description = description;
labels = LabelHelper.RemoveInvalidLabels(SanitizationHelper.SanitizeString(labels));
labels = LabelHelper.RemoveInvalidLabels(labels);
if (this.Slot.AuthorLabels != labels) this.Slot.AuthorLabels = labels;
// ReSharper disable once InvertIf

View file

@ -33,7 +33,6 @@ public class UserSettingsPage : BaseLayout
if (avatarHash != null) this.ProfileUser.IconHash = avatarHash;
biography = SanitizationHelper.SanitizeString(biography);
biography = CensorHelper.FilterMessage(biography);
if (this.ProfileUser.Biography != biography && biography.Length <= 512) this.ProfileUser.Biography = biography;

View file

@ -1,16 +1,17 @@
#nullable enable
using System.Collections.Generic;
using System.Reflection;
using System.Threading.Tasks;
using System.Web;
using LBPUnion.ProjectLighthouse.Database;
using LBPUnion.ProjectLighthouse.Helpers;
using LBPUnion.ProjectLighthouse.Types.Maintenance;
namespace LBPUnion.ProjectLighthouse.Administration.Maintenance.MigrationTasks;
public class CleanupXmlInjectionMigration : IMigrationTask
public class CleanupSanitizedStrings : IMigrationTask
{
public string Name() => "Cleanup XML injections";
public string Name() => "Cleanup Sanitized strings";
// Weird, but required. Thanks, hejlsberg.
async Task<bool> IMigrationTask.Run(DatabaseContext database)
{
List<object> objsToBeSanitized = new();
@ -25,7 +26,24 @@ public class CleanupXmlInjectionMigration : IMigrationTask
objsToBeSanitized.AddRange(database.Photos);
objsToBeSanitized.AddRange(database.Reports);
foreach (object obj in objsToBeSanitized) SanitizationHelper.SanitizeStringsInClass(obj);
foreach (object obj in objsToBeSanitized)
{
PropertyInfo[] properties = obj.GetType().GetProperties();
foreach (PropertyInfo property in properties)
{
if (property.PropertyType != typeof(string)) continue;
string? before = (string?)property.GetValue(obj);
if (before == null) continue;
string after = HttpUtility.HtmlDecode(before);
if (before != after)
{
property.SetValue(obj, after);
}
}
}
await database.SaveChangesAsync();
return true;

View file

@ -120,7 +120,6 @@ public static partial class ControllerExtensions
}
XmlSerializer serializer = new(typeof(T), root);
T? obj = (T?)serializer.Deserialize(new StringReader(bodyString));
SanitizationHelper.SanitizeStringsInClass(obj);
return obj;
}
catch (Exception e)

View file

@ -1,49 +1,9 @@
#nullable enable
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Reflection;
namespace LBPUnion.ProjectLighthouse.Helpers;
public static class SanitizationHelper
{
private static readonly Dictionary<string, string> charsToReplace = new() {
{"<", "&lt;"},
{">", "&gt;"},
{"\"", "&quot;"},
{"'", "&apos;"},
};
public static bool IsValidEmail(string? email) => !string.IsNullOrWhiteSpace(email) && new EmailAddressAttribute().IsValid(email);
public static void SanitizeStringsInClass(object? instance)
{
if (instance == null) return;
PropertyInfo[] properties = instance.GetType().GetProperties();
foreach (PropertyInfo property in properties)
{
if (property.PropertyType != typeof(string)) continue;
string? before = (string?) property.GetValue(instance);
if (before == null) continue;
if (!charsToReplace.Keys.Any(k => before.Contains(k))) continue;
property.SetValue(instance, SanitizeString(before));
}
}
public static string SanitizeString(string? input)
{
if (input == null) return "";
foreach ((string? key, string? value) in charsToReplace)
{
input = input.Replace(key, value);
}
return input;
}
}