An old server exploit causes infinite loop in serverloop thread.

[AxeelAnder]

I'm not sure if there's any issue has discussed about this.

If someone continuously and quickly connects and disconnects to server, server will enter infinite loop.

Still viable on newest tshock.

Though it can be easily defended by checking the frequency of connecting.

The true reason why it so is:

We can check it in the loop, if it is 0, just break the loop. It works as I tested.

I highly recommend to fix this because I really don't know how can this exploit be used, dos attack might not be needed.

[sr229]

Please do not disclose security issues in GitHub @AxeelAnder. Please contact @hakusaro directly in their emails regarding security exploits as this would allow hackers to exploit this vulnerability when everyone is disclosed about it in the open.

[bartico6]

This exploit is public and was disclosed/documented by myself in 2017.

[bartico6]

Additionally, to further explain the problem (I think you have drawn some false conclusions here @AxeelAnder):

This exploit is known as a zero-length exploit and has already been picked up a while ago, that other time being used to send messages with invalid lengths to lag out servers.

I think it was already addressed by @QuiCM because there was (I think?) already a check somewhere that would catch packets with headers that indicate the packet is <2 bytes long.

This has nothing to do w/ connection frequency. This is used by players with spambots to try and boot servers - spamming connections with 0-length packets is more effective because more net threads players cause the net thread to be occupied for longer (& more errors from plugins [in a specific version of this exploit] appear in console locking up the server more)

cc @hakusaro @QuiCM on that. I think this was already mentioned & fixed, no? If so, seems like it resurfaced.

[AxeelAnder]

@bartico6 We addressed that zero-length exploit together so i'm also familiar with it.
To solve that we check the length in MessageBuffer.GetData(or on OTAPI.Callbacks.Terraria.MessageBuffer.RecieveData), but this exploit happens in NetMessage.CheckBytes, it's before our check execution and the loop wouldn't stop even we checked it so it will be no use.

I'm not sure, maybe you're right.

[bartico6]

Then the length check should be moved up to be earlier in the ctl flow, that's the simplest solution I see.