Sweetgum/sweetgum-server
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Disconnect client by throwing exception in handler

Browse Source 
This makes it easier to add checks in handlers, which should improve security over time.
I think this approach is more readable and testable than calling Client#disconnect straight up,
while it also decentralizes the handling.
This commit is contained in:
P0nk
2023-08-06 15:48:49 +02:00
parent e9819fac87
commit 2686b2b02d
36 changed files with 180 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/main/java/client/Client.java
View File

@@ -29,6 +29,8 @@ import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.handler.timeout.IdleStateEvent;
import net.PacketHandler;
import net.PacketProcessor;
import net.netty.DisconnectException;
import net.netty.GameViolationException;
import net.netty.InvalidPacketHeaderException;
import net.packet.InPacket;
import net.packet.Packet;
@@ -201,6 +203,8 @@ public class Client extends ChannelInboundHandlerAdapter {
try {
MonitoredChrLogger.logPacketIfMonitored(this, opcode, packet.getBytes());
handler.handlePacket(packet, this);
} catch (GameViolationException gve) {
throw new DisconnectException(this, gve.getMessage());
} catch (final Throwable t) {
final String chrInfo = player != null ? player.getName() + " on map " + player.getMapId() : "?";
log.warn("Error in packet handler {}. Chr {}, account {}. Packet: {}", handler.getClass().getSimpleName(),
@@ -229,6 +233,8 @@ public class Client extends ChannelInboundHandlerAdapter {
SessionCoordinator.getInstance().closeSession(this, true);
} else if (cause instanceof IOException) {
closeMapleSession();
} else {
ctx.fireExceptionCaught(cause);
}
}
@@ -1018,7 +1024,7 @@ public class Client extends ChannelInboundHandlerAdapter {
//getChannelServer().removePlayer(player); already being done
player.cancelAllDebuffs();
player.saveCharToDB(true);
player.saveCharToDB();
player.logOff();
if (YamlConfig.config.server.INSTANT_NAME_CHANGE) {

11
src/main/java/client/processor/npc/DueyProcessor.java
View File

@@ -35,6 +35,7 @@ import client.inventory.manipulator.KarmaManipulator;
import config.YamlConfig;
import constants.id.ItemId;
import constants.inventory.ItemConstants;
import net.netty.GameViolationException;
import net.server.channel.Channel;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -296,24 +297,22 @@ public class DueyProcessor {
if (sendMessage != null && sendMessage.length() > 100) {
AutobanFactory.PACKET_EDIT.alert(c.getPlayer(), c.getPlayer().getName() + " tried to packet edit with Quick Delivery on duey.");
log.warn("Chr {} tried to use duey with too long of a text", c.getPlayer().getName());
c.disconnect(true, false);
return;
throw GameViolationException.textLength(sendMessage);
}
if (!quick) {
fee += 5000;
} else if (!c.getPlayer().haveItem(ItemId.QUICK_DELIVERY_TICKET)) {
AutobanFactory.PACKET_EDIT.alert(c.getPlayer(), c.getPlayer().getName() + " tried to packet edit with Quick Delivery on duey.");
log.warn("Chr {} tried to use duey with Quick Delivery without a ticket, mesos {} and amount {}", c.getPlayer().getName(), sendMesos, amount);
c.disconnect(true, false);
return;
throw new GameViolationException("Send Duey quick delivery without having a ticket");
}
long finalcost = (long) sendMesos + fee;
if (finalcost < 0 || finalcost > Integer.MAX_VALUE || (amount < 1 && sendMesos == 0)) {
AutobanFactory.PACKET_EDIT.alert(c.getPlayer(), c.getPlayer().getName() + " tried to packet edit with duey.");
log.warn("Chr {} tried to use duey with mesos {} and amount {}", c.getPlayer().getName(), sendMesos, amount);
c.disconnect(true, false);
return;
throw new GameViolationException("Send invalid mesos with duey");
}
if(c.getPlayer().getMeso() < finalcost) {

7
src/main/java/client/processor/npc/StorageProcessor.java
View File

@@ -32,6 +32,7 @@ import client.inventory.manipulator.KarmaManipulator;
import config.YamlConfig;
import constants.id.ItemId;
import constants.inventory.ItemConstants;
import net.netty.GameViolationException;
import net.packet.InPacket;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -69,8 +70,7 @@ public class StorageProcessor {
if (slot < 0 || slot > storage.getSlots()) { // removal starts at zero
AutobanFactory.PACKET_EDIT.alert(c.getPlayer(), c.getPlayer().getName() + " tried to packet edit with storage.");
log.warn("Chr {} tried to work with storage slot {}", c.getPlayer().getName(), slot);
c.disconnect(true, false);
return;
throw new GameViolationException("Withdraw item from invalid slot");
}
slot = storage.getSlot(InventoryType.getByType(type), slot);
@@ -128,8 +128,7 @@ public class StorageProcessor {
AutobanFactory.PACKET_EDIT.alert(c.getPlayer(),
c.getPlayer().getName() + " tried to packet edit with storage.");
log.warn("Chr {} tried to store item at slot {}", c.getPlayer().getName(), slot);
c.disconnect(true, false);
return;
throw new GameViolationException("Store item at invalid slot");
}
if (hasGMRestrictions(chr)) {

5
src/main/java/client/processor/stat/AssignAPProcessor.java
View File

@@ -31,6 +31,7 @@ import client.inventory.InventoryType;
import client.inventory.Item;
import config.YamlConfig;
import constants.skills.*;
import net.netty.GameViolationException;
import net.packet.InPacket;
import tools.PacketCreator;
import tools.Randomizer;
@@ -385,9 +386,7 @@ public class AssignAPProcessor {
} else {
if (inPacket.available() < 16) {
AutobanFactory.PACKET_EDIT.alert(chr, "Didn't send full packet for Auto Assign.");
c.disconnect(true, false);
return;
throw new GameViolationException("Auto Assign packet is too small");
}
for (int i = 0; i < 2; i++) {

5
src/main/java/client/processor/stat/AssignSPProcessor.java
View File

@@ -30,6 +30,7 @@ import client.SkillFactory;
import client.autoban.AutobanFactory;
import constants.game.GameConstants;
import constants.skills.Aran;
import net.netty.GameViolationException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import tools.PacketCreator;
@@ -50,9 +51,7 @@ public class AssignSPProcessor {
if ((!GameConstants.isPqSkillMap(player.getMapId()) && GameConstants.isPqSkill(skillid)) || (!player.isGM() && GameConstants.isGMSkills(skillid)) || (!GameConstants.isInJobTree(skillid, player.getJob().getId()) && !player.isGM())) {
AutobanFactory.PACKET_EDIT.alert(player, "tried to packet edit in distributing sp.");
log.warn("Chr {} tried to use skill {} without it being in their job.", c.getPlayer().getName(), skillid);
c.disconnect(true, false);
return false;
throw new GameViolationException("Assign SP into invalid skill");
}
return true;