GeyserMC / MCProtocolLib

A library for communication with a Minecraft client/server.
MIT License
724 stars 201 forks source link

Bug in parsing of chunk data? #745

Open artnaseef opened 1 year ago

artnaseef commented 1 year ago

Getting this stack trace on call to MinecraftCodecHelper.readChunkSection() with data from ClientboundLevelChunkWithLightPacket.getChunkData(). The packet parse is coming from a stock, vanilla Minecraft 1.20.1 server:

java.lang.IndexOutOfBoundsException: readerIndex(36599) + length(1) exceeds writerIndex(36599): 
UnpooledHeapByteBuf(ridx: 36599, widx: 36599, cap: 36599/36599)
        at io.netty.buffer.AbstractByteBuf.checkReadableBytes0(AbstractByteBuf.java:1477)
        at io.netty.buffer.AbstractByteBuf.readByte(AbstractByteBuf.java:731)
        at com.github.steveice10.packetlib.codec.BasePacketCodecHelper.readVarInt(BasePacketCodecHelper.java:20)
        at com.github.steveice10.mc.protocol.codec.MinecraftCodecHelper.readDataPalette(MinecraftCodecHelper.java:689)
        at com.github.steveice10.mc.protocol.codec.MinecraftCodecHelper.readChunkSection(MinecraftCodecHelper.java:738)

Debugging it, the cause appears to be an attempt to read past the end of the buffer here:

https://github.com/GeyserMC/MCProtocolLib/blob/master/src/main/java/com/github/steveice10/mc/protocol/codec/MinecraftCodecHelper.java#L689

Noticed the following code is new:

// Eat up - can be seen on Hypixel as of 1.19.0
int length = this.readVarInt(buf);
for (int i = 0; i < length; i++) {
    buf.readLong();
}
storage = null;

Changing it as follows fixes my problem. Looking for advice here.

// Eat up - can be seen on Hypixel as of 1.19.0
if (buf.readableBytes() > 0) {
    int length = this.readVarInt(buf);
    for (int i = 0; i < length; i++) {
        buf.readLong();
    }
}

storage = null;

Thoughts?

AlexProgrammerDE commented 2 weeks ago

Still happening?

artnaseef commented 2 weeks ago

Honestly - not sure where this is now. I haven't looked at it in a year. This is the first feedback I've gotten.

AlexProgrammerDE commented 2 weeks ago

Recently someone has contributed some changes that fix some weird chunk behaviours. Maybe this is the same issue and it's fixed now. Or this is something else.

artnaseef commented 2 weeks ago

OK, I'll take a look. I need to try to get to the latest version of Minecraft, so this is a good time to check it out. I should be able to work on it this week.

artnaseef commented 1 week ago

Turned out to be a super busy week. Hoping this coming week is better.