Fabric Mod Sender Integration. Mod itself is not final #307
Keine Reviewer
Label
Kein Label
Bug
Codeverbesserung
Einsteiger Freundlich
Idee
In Arbeit
Neues Feature
Prio A
Security Breach
Überprüfung notwendig
Verbesserung
Zu Beobachten
Kein Meilenstein
Niemand zuständig
2 Beteiligte
Fällig am
Kein Fälligkeitsdatum gesetzt.
Abhängigkeiten
Keine Abhängigkeiten gesetzt.
Referenz: SteamWar/BungeeCore#307
Laden…
In neuem Issue referenzieren
Einen Benutzer sperren
Keine Beschreibung angegeben.
Branch "fabric_mod_sender" löschen
Das Löschen eines Branches ist permanent. Obwohl der Branch für eine kurze Zeit weiter existieren könnte, kann diese Aktion in den meisten Fällen NICHT rückgängig gemacht werden. Fortfahren?
Mod has to get a tiny bit of updating. But this part works non the less.
Ich würde mir wünschen, dass noch überprüft wird, dass das Packet genau so formatiert ist/aussieht, als würde es von einem unmodifizierten FabricModChecker geschickt werden. Sollten dabei Abweichungen auffallen, sollte es sofort zu einem automatisierten Permabann + WARNING-Lognachricht kommen (da dann jemand versucht, den FabricModChecker zu umgehen).
Folgende Dinge sollten dabei auf Abweichungen geprüft werden:
@ -0,0 +1,45 @@
package de.steamwar.bungeecore.listeners.mods;
License-Header.
@ -0,0 +26,4 @@
dataString = dataString.replace(x, ' ');
dataString = dataString.replaceAll("\\s+","");
String[] parts = dataString.split(",");
Das Stringhandling schaut mir ziemlich Spanisch aus, ist das hier ein sehr primitiver JSON-Parser?
@ -0,0 +34,4 @@
mods.add(Mod.get(mod, Mod.Platform.FABRIC));
}
if (e.getSender() instanceof ProxiedPlayer) {
Das bitte zu Anfang prüfen (und ggf. verwerfen), wenn das Packet vom Subserver kommt, ist das ja uninteressant für uns.
@ -0,0 +38,4 @@
ProxiedPlayer player = (ProxiedPlayer) e.getSender();
if(!Utils.handleMods(player, mods))
player.disconnect("");
Ich glaube, das disconnecten musst du nicht mehr von Hand machen (kannst ja mal im LabymodHandler oder Forge1.12Handler nachschauen (Forge 1.13+ ist da etwas spezieller weil in LOGIN)
@ -0,0 +39,4 @@
if(!Utils.handleMods(player, mods))
player.disconnect("");
}
Sollten die Mods einwandfrei sein, sollte der Spieler auf eine Whitelist kommen, dass er auch Arenenserver betreten kann (siehe ModLoaderBlocker). Dazu am Besten den Spieler eine Message senden á'la "Deine Mods wurden überprüft, du darfst Arenen betreten".
@ -0,0 +41,4 @@
public class Fabric extends BasicListener {
private final List<String> neededMods = new LinkedList<>();
Warum ist das eine LinkedList? Wäre der korrekte Datentyp hier nicht ein Set?
@ -0,0 +67,4 @@
byte[] data = e.getData();
Utils.VarInt varInt = Utils.readVarInt(data,0);
data = Arrays.copyOfRange(data,varInt.length, data.length);
Du könntest das Packet prinzipiell vereinfachen, indem du den String ohne Länge vorher schickst. Der String wäre dann das gesamte Packet. Ansonsten würde ich mir wünschen, dass du überprüfst, dass der VarInt tatsächlich die Länge des restlichen Packets enthält (und falls Fehler Bann), derzeit nutzt du ja den VarInt gar nicht.
1 teres kann ich nicht tun, da dieser varint von minecraft direkt angepappt wird. zweites kann ich machen.
@ -0,0 +69,4 @@
data = Arrays.copyOfRange(data,varInt.length, data.length);
String dataString = new String(data, StandardCharsets.UTF_8);
Hier kann eine CharsetException oder ähnliches auftreten, in dem Fall wurde (offensichtlich) auch am fabricmodsender herumgepfuscht.
@ -0,0 +75,4 @@
JsonArray array = new JsonParser().parse(dataString).getAsJsonArray();
for(int i = 0; i < array.size(); i++) {
mods.add(Mod.get(array.get(i).getAsString(), Mod.Platform.FABRIC));
Kannst du hier nicht for(X modIdentifier : array) machen?
@ -0,0 +82,4 @@
if(!isSorted) {
SteamwarUser user = SteamwarUser.get(player.getUniqueId());
user.punish(Punishment.PunishmentType.Ban, Timestamp.from(Instant.now()), "§7Du hast probiert den FabricModSender zu umgehen!", 0, true);
Das würde ich mir einmal Multilingual mit Message.parse(Prefixless?)() wünschen.
@ -0,0 +90,4 @@
if(!modsAreAbsent(mods)) {
SteamwarUser user = SteamwarUser.get(player.getUniqueId());
user.punish(Punishment.PunishmentType.Ban, Timestamp.from(Instant.now()), "§7Du hast probiert den FabricModSender zu umgehen!", 0, true);
BungeeCore.log(Level.SEVERE, user.getUserName() + " " + user.getId() + " wurde automatisch gebannt, da er den FabricModSender editiert hatte.");
Diese drei Codezeilen hast du doppelt drin, daher würde ich mir wünschen, dass du die in eine Funktion auslagerst.
@ -0,0 +94,4 @@
return;
}
if(Utils.handleMods(player,mods)) checkedPlayers.add(player);
Du solltest lieber den Spieler aus den Storage.fabricPlayers entfernen (und den Spieler darüber informieren, dass die Mods geprüft wurden und er deshalb auf Arenen darf). Das derzeitige System hat das Problem, sobald ein softreload gemacht wird, können geprüfte Spieler eben nicht mehr auf Arenen.
@ -0,0 +95,4 @@
}
if(Utils.handleMods(player,mods)) checkedPlayers.add(player);
}catch (Exception ex) {
Das Try-Catch gefällt mir gar nicht. Mir ist bewusst, dass beim JsonParser Fehler fliegen können, in dem Fall solltest du aber nur den JsonParser try-catchen und falls das auftritt auch wieder bannen.
@ -0,0 +124,4 @@
return isSorted;
}
private boolean modsAreAbsent(List<Mod> mods) {
Der Methodenname ist irreführend, denn der zurückgegebene Boolean ist ja, dass kein Mod fehlt.
@ -0,0 +128,4 @@
return mods.stream()
.map(Mod::getModName)
.filter(neededMods::contains)
.distinct()
Distinct darf hier nicht nötig sein!
@ -0,0 +138,4 @@
Message.parse("MODIFICATION_BAN_MESSAGE", player),
0,
true);
BungeeCore.log(Level.SEVERE, user.getUserName() + " " + user.getId() + Message.parse("MODIFICATION_BAN_LOG", player));
Änderung siehe BungeeCore.properties, Message.parse(..., player, user.getUserName(), user.getId())
@ -586,0 +586,4 @@
#Fabric Mod Sender
MODIFICATION_BAN_MESSAGE = "Du hast probiert den FabricModSender zu umgehen / zu modifizieren!"
MODIFICATION_BAN_LOG = " hat probiert den Fabric Mod Sender zu editieren / umzugehen!"
Keine Leerzeichen um das =. Keine " um den String herum. Statt dem " hat" solltest du "{0} {1} hat" verwenden (dann übergibst du das noch als Parameter und überlässt die Formatierung dem MessageSystem.
@ -0,0 +137,4 @@
Message.parse("MODIFICATION_BAN_MESSAGE", player, user.getUserName(), user.getId()),
0,
true);
BungeeCore.log(Level.SEVERE, user.getUserName() + " " + user.getId() + Message.parse("MODIFICATION_BAN_LOG", player, user.getUserName()));
Hier den String nicht mehr zusammenaddieren (Message.parse-Ergebnis enthält ja schon alles)
@ -585,1 +585,4 @@
RANK_NEEDED_FIGHTS_LEFT={0} §8(§7noch §e{1}§7 Kämpfe nötig§8)
#Fabric Mod Sender
MODIFICATION_BAN_MESSAGE="Du hast probiert den FabricModSender zu umgehen / zu modifizieren!"
Immer noch: "" entfernen! Und in der Zeile drunter das leerzeichen