diff --git a/Content.Client/Instruments/MidiParser/MidiParser.cs b/Content.Client/Instruments/MidiParser/MidiParser.cs index 937384e439..0b65e472fc 100644 --- a/Content.Client/Instruments/MidiParser/MidiParser.cs +++ b/Content.Client/Instruments/MidiParser/MidiParser.cs @@ -102,6 +102,8 @@ public static class MidiParser // 0x03 is TrackName, // 0x04 is InstrumentName + // This string can potentially contain control characters, including 0x00 which can cause problems if it ends up in database entries via admin logs + // we sanitize TrackName and InstrumentName after they have been send to the server var text = Encoding.ASCII.GetString(metaData, 0, (int)metaLength); switch (metaType) { diff --git a/Content.Server/Instruments/InstrumentSystem.cs b/Content.Server/Instruments/InstrumentSystem.cs index 420bd44737..f6a2162271 100644 --- a/Content.Server/Instruments/InstrumentSystem.cs +++ b/Content.Server/Instruments/InstrumentSystem.cs @@ -156,6 +156,15 @@ public sealed partial class InstrumentSystem : SharedInstrumentSystem return; } + + foreach (var t in msg.Tracks) + { + // Remove any control characters that may be part of the midi file so they don't end up in the admin logs. + t?.SanitizeFields(); + // Truncate any track names too long. + t?.TruncateFields(_cfg.GetCVar(CCVars.MidiMaxChannelNameLength)); + } + var tracksString = string.Join("\n", msg.Tracks .Where(t => t != null) @@ -166,12 +175,6 @@ public sealed partial class InstrumentSystem : SharedInstrumentSystem LogImpact.Low, $"{ToPrettyString(args.SenderSession.AttachedEntity)} set the midi channels for {ToPrettyString(uid)} to {tracksString}"); - // Truncate any track names too long. - foreach (var t in msg.Tracks) - { - t?.TruncateFields(_cfg.GetCVar(CCVars.MidiMaxChannelNameLength)); - } - activeInstrument.Tracks = msg.Tracks; Dirty(uid, activeInstrument); diff --git a/Content.Shared/Instruments/SharedInstrumentComponent.cs b/Content.Shared/Instruments/SharedInstrumentComponent.cs index 97eef752eb..41bef64902 100644 --- a/Content.Shared/Instruments/SharedInstrumentComponent.cs +++ b/Content.Shared/Instruments/SharedInstrumentComponent.cs @@ -1,4 +1,5 @@ using System.Collections; +using System.Text; using Robust.Shared.Audio.Midi; using Robust.Shared.GameStates; using Robust.Shared.Serialization; @@ -207,6 +208,18 @@ public sealed class MidiTrack ProgramName = Truncate(ProgramName, limit); } + public void SanitizeFields() + { + if (InstrumentName != null) + InstrumentName = Sanitize(InstrumentName); + + if (TrackName != null) + TrackName = Sanitize(TrackName); + + if (ProgramName != null) + ProgramName = Sanitize(ProgramName); + } + private const string Postfix = "…"; // TODO: Make a general method to use in RT? idk if we have that. private string Truncate(string input, int limit) @@ -218,4 +231,17 @@ public sealed class MidiTrack return input.Substring(0, truncatedLength) + Postfix; } + + private static string Sanitize(string input) + { + var sanitized = new StringBuilder(input.Length); + + foreach (char c in input) + { + if (!char.IsControl(c) && c <= 127) // no control characters, only ASCII + sanitized.Append(c); + } + + return sanitized.ToString(); + } }