From e3cfa45607d2bf4a8ad4a4d684a6889ef15b586a Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 12 Mar 2013 02:02:36 +0100 Subject: [PATCH] Ensure that monitor listeners cannot modify a packet event. --- .../protocol/events/PacketEvent.java | 34 ++++++++++++++++++- .../injector/SortedPacketListenerList.java | 20 +++++------ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java index c35330cb..5d43ff50 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java @@ -44,6 +44,9 @@ public class PacketEvent extends EventObject implements Cancellable { private AsyncMarker asyncMarker; private boolean asynchronous; + // Whether or not a packet event is read only + private boolean readOnly; + /** * Use the static constructors to create instances of this event. * @param source - the event source. @@ -114,6 +117,8 @@ public class PacketEvent extends EventObject implements Cancellable { * @param packet - the packet that will be sent instead. */ public void setPacket(PacketContainer packet) { + if (readOnly) + throw new IllegalStateException("The packet event is read-only."); this.packet = packet; } @@ -147,6 +152,8 @@ public class PacketEvent extends EventObject implements Cancellable { * @param cancel - TRUE if it should be cancelled, FALSE otherwise. */ public void setCancelled(boolean cancel) { + if (readOnly) + throw new IllegalStateException("The packet event is read-only."); this.cancel = cancel; } @@ -193,9 +200,34 @@ public class PacketEvent extends EventObject implements Cancellable { public void setAsyncMarker(AsyncMarker asyncMarker) { if (isAsynchronous()) throw new IllegalStateException("The marker is immutable for asynchronous events"); + if (readOnly) + throw new IllegalStateException("The packet event is read-only."); this.asyncMarker = asyncMarker; } + /** + * Determine if the current packet event is read only. + *

+ * This is used to ensure that a monitor listener doesn't accidentally alter the state of the event. However, + * it is still possible to modify the packet itself, as it would require too many resources to verify its integrity. + *

+ * Thus, the packet is considered immutable if the packet event is read only. + * @return TRUE if it is, FALSE otherwise. + */ + public boolean isReadOnly() { + return readOnly; + } + + /** + * Set the read-only state of this packet event. + *

+ * This will be reset for every packet listener. + * @param readOnly - TRUE if it is read-only, FALSE otherwise. + */ + public void setReadOnly(boolean readOnly) { + this.readOnly = readOnly; + } + /** * Determine if the packet event has been executed asynchronously or not. * @return TRUE if this packet event is asynchronous, FALSE otherwise. @@ -203,7 +235,7 @@ public class PacketEvent extends EventObject implements Cancellable { public boolean isAsynchronous() { return asynchronous; } - + private void writeObject(ObjectOutputStream output) throws IOException { // Default serialization output.defaultWriteObject(); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/SortedPacketListenerList.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/SortedPacketListenerList.java index 9862b3ca..53fe6366 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/SortedPacketListenerList.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/SortedPacketListenerList.java @@ -45,6 +45,7 @@ public final class SortedPacketListenerList extends AbstractConcurrentListenerMu // The returned list is thread-safe for (PrioritizedListener element : list) { try { + event.setReadOnly(element.getPriority() == ListenerPriority.MONITOR); element.getListener().onPacketReceiving(event); } catch (Throwable e) { // Minecraft doesn't want your Exception. @@ -67,15 +68,14 @@ public final class SortedPacketListenerList extends AbstractConcurrentListenerMu return; for (PrioritizedListener element : list) { - final PacketListener listener = element.getListener(); - try { - if (listener.getReceivingWhitelist().getPriority() == priorityFilter) { - listener.onPacketReceiving(event); + if (element.getPriority() == priorityFilter) { + event.setReadOnly(element.getPriority() == ListenerPriority.MONITOR); + element.getListener().onPacketReceiving(event); } } catch (Throwable e) { // Minecraft doesn't want your Exception. - reporter.reportMinimal(listener.getPlugin(), "onPacketReceiving(PacketEvent)", e, + reporter.reportMinimal(element.getListener().getPlugin(), "onPacketReceiving(PacketEvent)", e, event.getPacket().getHandle()); } } @@ -94,6 +94,7 @@ public final class SortedPacketListenerList extends AbstractConcurrentListenerMu for (PrioritizedListener element : list) { try { + event.setReadOnly(element.getPriority() == ListenerPriority.MONITOR); element.getListener().onPacketSending(event); } catch (Throwable e) { // Minecraft doesn't want your Exception. @@ -116,15 +117,14 @@ public final class SortedPacketListenerList extends AbstractConcurrentListenerMu return; for (PrioritizedListener element : list) { - final PacketListener listener = element.getListener(); - try { - if (listener.getSendingWhitelist().getPriority() == priorityFilter) { - listener.onPacketSending(event); + if (element.getPriority() == priorityFilter) { + event.setReadOnly(element.getPriority() == ListenerPriority.MONITOR); + element.getListener().onPacketSending(event); } } catch (Throwable e) { // Minecraft doesn't want your Exception. - reporter.reportMinimal(listener.getPlugin(), "onPacketSending(PacketEvent)", e, + reporter.reportMinimal(element.getListener().getPlugin(), "onPacketSending(PacketEvent)", e, event.getPacket().getHandle()); } }