From 4e5b17ce4e948a42cbdfd57420249da4ff8e88ea Mon Sep 17 00:00:00 2001 From: robert <> Date: Sun, 24 Jan 2021 08:47:21 +0000 Subject: [PATCH 1/4] Fix /pass bypass security issue. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mitigation: If verify-names is enabled the authentication is handled by the TPP that server overrides the username processing. The default server, classicube.net, uses case sensitive names with the initial logon mapping the entered userid to the correct case. It is expected that alternate servers will also not allow user names that collide under case conversion. Issue: The username is usually treated as case insensitive, however, the password is stored in a file with the user name in it's name. For filesystems that ignore case this will only occasionally an issue if unicode case conversion is inconsistent. But linux filesystems do not ignore case so all letters (and letter like characters) in the username have two representations. Contributory factor: If the password file for a particular userid is not found the user is prompted to enter a new password without any authentication. This update has been setup to recreate the password files using a new name including the lowercase username. Because of this two other issues can be fixed. The use of MD5. While still okay for passwords MD5 should be replaced at the first reasonable opportunity. I have used SHA2-256 Unicode password characters; these were all replaced by the character "?", this means that a password like "пароль" would give an identical hash to "??????". --- MCGalaxy/Commands/Moderation/CmdPass.cs | 83 +++++++++++++++++++++++-- MCGalaxy/Player/Player.cs | 2 +- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/MCGalaxy/Commands/Moderation/CmdPass.cs b/MCGalaxy/Commands/Moderation/CmdPass.cs index 257baf534..f3f4a6e0d 100644 --- a/MCGalaxy/Commands/Moderation/CmdPass.cs +++ b/MCGalaxy/Commands/Moderation/CmdPass.cs @@ -21,6 +21,10 @@ using System; using System.IO; using System.Security.Cryptography; using System.Text; +#if !DISABLE_OLD_PASSWORDFILE +using System.Collections.Generic; +using System.Linq; +#endif namespace MCGalaxy.Commands.Moderation { public sealed class CmdPass : Command2 { @@ -66,6 +70,17 @@ namespace MCGalaxy.Commands.Moderation { p.Message("You are now &averified %Sand have &aaccess to admin commands and features."); p.verifiedPass = true; p.Unverified = false; + +#if !DISABLE_OLD_PASSWORDFILE + string oldpath = HashPath(p.name, false); + string path = HashPath(p.name, true); + if (oldpath != path) { + File.Delete(oldpath); + byte[] computed = ComputeHash(p.name, password); + File.WriteAllBytes(path, computed); + p.Message("Your password was &areset for the new format"); + } +#endif } else { p.passtries++; p.Message("%WWrong Password. %SRemember your password is %Wcase sensitive."); @@ -108,16 +123,16 @@ namespace MCGalaxy.Commands.Moderation { } } - static string HashPath(string name) { return "extra/passwords/" + name + ".dat"; } - static bool HasPassword(string name) { return File.Exists(HashPath(name)); } - - static byte[] ComputeHash(string name, string pass) { +#if !DISABLE_OLD_PASSWORDFILE + static byte[] OldComputeHash(string name, string pass) { // Pointless, but kept for backwards compatibility pass = pass.Replace("<", "("); pass = pass.Replace(">", ")"); MD5 hash = MD5.Create(); byte[] nameB = hash.ComputeHash(Encoding.ASCII.GetBytes(name)); + // This line means that non-ASCII characters in passwords are + // all encoded as the "?" character. byte[] dataB = hash.ComputeHash(Encoding.ASCII.GetBytes(pass)); byte[] result = new byte[nameB.Length + dataB.Length]; @@ -125,16 +140,40 @@ namespace MCGalaxy.Commands.Moderation { Array.Copy(dataB, 0, result, nameB.Length, dataB.Length); return hash.ComputeHash(result); } +#endif + + static byte[] ComputeHash(string name, string pass) { + // Pointless, but kept for backwards compatibility + pass = pass.Replace("<", "("); + pass = pass.Replace(">", ")"); + + // The constant string added to the username salt is to mitigate + // rainbox tables. We should really have a unique salt for each + // user, but this is close enough. + return SHA256.Create().ComputeHash(Encoding.UTF8.GetBytes("MCGalaxy:" + name.ToLower() + " " + pass)); + } static bool CheckHash(string name, string pass) { byte[] stored = File.ReadAllBytes(HashPath(name)); byte[] computed = ComputeHash(name, pass); + +#if !DISABLE_OLD_PASSWORDFILE + // Old style MD5 passwords. + byte[] oldcomputed = OldComputeHash(name, pass); + if (stored.Length == oldcomputed.Length) { + bool oldokay = true; + for (int i = 0; i < stored.Length; i++) { + if (stored[i] != oldcomputed[i]) oldokay = false; + } + if (oldokay) return true; + } + // Old passwords stored UTF8 string instead of just the raw 16 byte hashes // We need to support both since this behaviour was accidentally changed if (stored.Length != computed.Length) { return Encoding.UTF8.GetString(stored) == Encoding.UTF8.GetString(computed); } - +#endif for (int i = 0; i < stored.Length; i++) { if (stored[i] != computed[i]) return false; } @@ -150,5 +189,39 @@ namespace MCGalaxy.Commands.Moderation { p.Message("%HIf you are an admin, use this command to verify your login."); p.Message("%H You will need to be verified to be able to use commands."); } + + public static bool HasPassword(string name) { return File.Exists(HashPath(name)); } + +#if DISABLE_OLD_PASSWORDFILE + static string HashPath(string name) { return "extra/passwords/" + name.ToLower() + ".pwd"; } +#else + static string HashPath(string name, bool ForUpdate = false) + { + string PassName = "extra/passwords/" + name.ToLower() + ".pwd"; + if (ForUpdate || File.Exists(PassName)) + return PassName; + + string OldName = "extra/passwords/" + name + ".dat"; + string directory = Path.GetDirectoryName(OldName); + IEnumerable foundFiles = EnumerateFilesCI(directory, OldName); + + if (foundFiles.Count() < 1) + return PassName; + + return foundFiles.First(); + } + + private static IEnumerable EnumerateFilesCI(string directory, string pathAndFileName) + { + foreach (string file in Directory.EnumerateFiles(directory)) + { + if (String.Equals(file, pathAndFileName, StringComparison.OrdinalIgnoreCase) ) + { + yield return file; + } + } + } +#endif + } } diff --git a/MCGalaxy/Player/Player.cs b/MCGalaxy/Player/Player.cs index 5c7819aa1..49407fb62 100644 --- a/MCGalaxy/Player/Player.cs +++ b/MCGalaxy/Player/Player.cs @@ -357,7 +357,7 @@ namespace MCGalaxy { Unverified = Server.Config.verifyadmins && Rank >= Server.Config.VerifyAdminsRank; if (!Unverified) return; - if (!File.Exists("extra/passwords/" + name + ".dat")) { + if (!Commands.Moderation.CmdPass.HasPassword(name)) { Message("%WPlease set your admin verification password with %T/SetPass [password]!"); } else { Message("%WPlease complete admin verification with %T/Pass [password]!"); From 32ab58d331bcdcd2336272ece245daf7700aea7f Mon Sep 17 00:00:00 2001 From: robert <> Date: Mon, 25 Jan 2021 08:30:18 +0000 Subject: [PATCH 2/4] Small tweak for "correct" old password file name --- MCGalaxy/Commands/Moderation/CmdPass.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MCGalaxy/Commands/Moderation/CmdPass.cs b/MCGalaxy/Commands/Moderation/CmdPass.cs index f3f4a6e0d..46dbd06c8 100644 --- a/MCGalaxy/Commands/Moderation/CmdPass.cs +++ b/MCGalaxy/Commands/Moderation/CmdPass.cs @@ -202,6 +202,9 @@ namespace MCGalaxy.Commands.Moderation { return PassName; string OldName = "extra/passwords/" + name + ".dat"; + if (File.Exists(PassName)) + return PassName; + string directory = Path.GetDirectoryName(OldName); IEnumerable foundFiles = EnumerateFilesCI(directory, OldName); From 4ffa3e663679b84b034a40324df5ba65252383d0 Mon Sep 17 00:00:00 2001 From: robert <> Date: Wed, 27 Jan 2021 13:49:34 +0000 Subject: [PATCH 3/4] Remove old backwards compatibility --- MCGalaxy/Commands/Moderation/CmdPass.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/MCGalaxy/Commands/Moderation/CmdPass.cs b/MCGalaxy/Commands/Moderation/CmdPass.cs index 46dbd06c8..a89dc3053 100644 --- a/MCGalaxy/Commands/Moderation/CmdPass.cs +++ b/MCGalaxy/Commands/Moderation/CmdPass.cs @@ -143,10 +143,6 @@ namespace MCGalaxy.Commands.Moderation { #endif static byte[] ComputeHash(string name, string pass) { - // Pointless, but kept for backwards compatibility - pass = pass.Replace("<", "("); - pass = pass.Replace(">", ")"); - // The constant string added to the username salt is to mitigate // rainbox tables. We should really have a unique salt for each // user, but this is close enough. From c9dacf3fc866e85924a434d90786004a01fbb113 Mon Sep 17 00:00:00 2001 From: robert <> Date: Sat, 30 Jan 2021 13:58:59 +0000 Subject: [PATCH 4/4] Alter salt extension to be obviously random. --- MCGalaxy/Commands/Moderation/CmdPass.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MCGalaxy/Commands/Moderation/CmdPass.cs b/MCGalaxy/Commands/Moderation/CmdPass.cs index a89dc3053..4ca47910a 100644 --- a/MCGalaxy/Commands/Moderation/CmdPass.cs +++ b/MCGalaxy/Commands/Moderation/CmdPass.cs @@ -146,7 +146,7 @@ namespace MCGalaxy.Commands.Moderation { // The constant string added to the username salt is to mitigate // rainbox tables. We should really have a unique salt for each // user, but this is close enough. - return SHA256.Create().ComputeHash(Encoding.UTF8.GetBytes("MCGalaxy:" + name.ToLower() + " " + pass)); + return SHA256.Create().ComputeHash(Encoding.UTF8.GetBytes("0bec662b-416f-450c-8f50-664fd4a41d49" + name.ToLower() + " " + pass)); } static bool CheckHash(string name, string pass) {