WorldDownloader can be crashed with a server-side plugin and the Plugin Messaging API about worlddownloader HOT 8 CLOSED

Yea, I'll fix this (probably by disabling the plugin message API if an exception occurs to discourage trying to crash it).

The problem is, of course, the vanilla client can be crashed with malformed packets, and if I recall correctly any issue about that is closed as "Works as intended". So it's not possible to stop all misuse unfortunately.

@Redrield: If you need help with forming a valid packet, I'd be happy to help here. I can also help in #mcdevs in a day or two, but I don't have access to an IRC client right now.

from worlddownloader.

So will you fix it or...? Sorry it just wasn't clear to me from your message.

@Redrield wants no part in this I reckon, as they created that plugin for their server and got a little annoyed that I told you this. AFAICT they don't want to make a valid packet, they just want to crash the client.

from worlddownloader.

I didn't say that I didn't want to make a malformed packet, in fact I made that plugin with all good intentions of just disabling the functionality of WDL when a packet in the WDL|INIT channel was received.

Some more notes about why this is happening, it only occurs when I send WDL|CONTROL0, if I send any other packets to the client, no crash occurs (They're also still able to use WDL, even if I send WDL|CONTROL1, saying that they have no permissions.

from worlddownloader.

Yea, I'll fix it.

@Redrield: Probably the crash happens from different internal formatting between the way WDL expects the message and the way Sponge does. You might have more success if you used a raw channel outbound too.

With WDL|CONTROL 1, is anything logged in chat if you turn on "Plugin channel message" in WDL's message options?

from worlddownloader.

@Pokechu22 These messages pop up when I do that:
[WorldDL] Sending plugin channel registration
[WorldDL] Receiving unknown settings packet #16777216. See log for binary contents

from worlddownloader.

One little update here, it seems that if I send WDL|CONTROL 0 through a raw data channel (Write the discriminator and boolean manually) it works, it's only the indexed channel that causes a crash

from worlddownloader.

#16777216 confirms what I thought – in hex it's 0x01000000. Most likely the 000000 part wasn't written as part of the discriminator, but instead the payload (your disabled permissions).

What that means is that Sponge is probably encoding the discriminator as a byte or VarInt, rather than a full into that WDL expects. I don't know Sponge well enough to know if you can change that directly or if your only option is to use raw channels.

The reason packet 0 caused a crash probably was the length of the packet: only the discriminator and a boolean (1 byte). If it was written with the discriminator as 1 byte as well, that's only 2 total bytes. And the discriminator is read as a 4 byte int, so that would fail to read, causing the crash.

On the other hand, packet 1 is long enough for the discriminator to read successfully read. And since it was read as 16777216, it doesn't recognize the packet, and reads no further (avoiding the crash that would happen if it were actually read due to length).

from worlddownloader.

Looking at the code for SpongeVanilla (The server I was using when I was testing this out) it looks like the message id passed gets converted to a byte, and then is mapped alongside the Class<? extends Message> that gets passed with it. https://github.com/SpongePowered/SpongeVanilla/blob/bleeding/src/main/java/org/spongepowered/server/network/VanillaIndexedMessageChannel.java#L67 is where the message gets registered, https://github.com/SpongePowered/SpongeVanilla/blob/bleeding/src/main/java/org/spongepowered/server/network/VanillaIndexedMessageChannel.java#L135 is where the packet gets created (The return value is just sent to the EntityPlayer's connection

from worlddownloader.